[llvm] r363292 - [LFTR] Stylistic cleanup as suggested in last review comment of D62939 [NFC]

[Florian Hahn via llvm-commits]

Hi,

I temporarily reverted the patch, because it depends on r363289, which breaks a green dragon
build:
   http://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/18208 <http://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/18208>

I’ll upload a reduced reproducer at https://bugs.llvm.org/show_bug.cgi?id=42279 as soon as bugpoint is done.

> On Jun 14, 2019, at 11:40, Florian Hahn via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Hi,
> 
> It looks like oss-fuzz started detecting an issue in indvar over the last day or 2: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=15237 <https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=15237> .
> 
>  I had a quick look over the changes in the range and it looks like this patch is one of the few that touched IndVarSimplify over that period (as well as r 363293). It would be great if you could have a quick look. In case I missed the real culprit, apologies for the noise!
> 
> Cheers,
> Florian
> 
>> On Jun 13, 2019, at 19:32, Philip Reames via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>> 
>> Author: reames
>> Date: Thu Jun 13 11:32:55 2019
>> New Revision: 363292
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=363292&view=rev <http://llvm.org/viewvc/llvm-project?rev=363292&view=rev>
>> Log:
>> [LFTR] Stylistic cleanup as suggested in last review comment of D62939 [NFC]
>> 
>> 
>> Modified:
>>    llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
>> 
>> Modified: llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp?rev=363292&r1=363291&r2=363292&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp?rev=363292&r1=363291&r2=363292&view=diff>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp Thu Jun 13 11:32:55 2019
>> @@ -2385,12 +2385,15 @@ bool IndVarSimplify::
>> linearFunctionTestReplace(Loop *L, BasicBlock *ExitingBB,
>>                           const SCEV *BackedgeTakenCount,
>>                           PHINode *IndVar, SCEVExpander &Rewriter) {
>> +  assert(isLoopCounter(IndVar, L, SE));
>> +  assert(L->getLoopLatch() && "Loop no longer in simplified form?");
>> +  Instruction * const IncVar =
>> +    cast<Instruction>(IndVar->getIncomingValueForBlock(L->getLoopLatch()));
>> +
>>   // Initialize CmpIndVar and IVCount to their preincremented values.
>>   Value *CmpIndVar = IndVar;
>>   const SCEV *IVCount = BackedgeTakenCount;
>> 
>> -  assert(L->getLoopLatch() && "Loop no longer in simplified form?");
>> -
>>   // If the exiting block is the same as the backedge block, we prefer to
>>   // compare against the post-incremented value, otherwise we must compare
>>   // against the preincremented value.
>> @@ -2401,12 +2404,10 @@ linearFunctionTestReplace(Loop *L, Basic
>>       // to add a potentially UB introducing use.  We need to either a) show
>>       // the loop test we're modifying is already in post-inc form, or b) show
>>       // that adding a use must not introduce UB.
>> -      Instruction *Inc =
>> -        cast<Instruction>(IndVar->getIncomingValueForBlock(L->getLoopLatch()));
>>       ICmpInst *LoopTest = getLoopTest(L, ExitingBB);
>> -      SafeToPostInc = LoopTest->getOperand(0) == Inc ||
>> -        LoopTest->getOperand(1) == Inc ||
>> -        mustExecuteUBIfPoisonOnPathTo(Inc, ExitingBB->getTerminator(), DT);
>> +      SafeToPostInc = LoopTest->getOperand(0) == IncVar ||
>> +        LoopTest->getOperand(1) == IncVar ||
>> +        mustExecuteUBIfPoisonOnPathTo(IncVar, ExitingBB->getTerminator(), DT);
>>     }
>>     if (SafeToPostInc) {
>>       // Add one to the "backedge-taken" count to get the trip count.
>> @@ -2417,7 +2418,7 @@ linearFunctionTestReplace(Loop *L, Basic
>>       // The BackedgeTaken expression contains the number of times that the
>>       // backedge branches to the loop header.  This is one less than the
>>       // number of times the loop executes, so use the incremented indvar.
>> -      CmpIndVar = IndVar->getIncomingValueForBlock(ExitingBB);
>> +      CmpIndVar = IncVar;
>>     }
>>   }
>> 
>> @@ -2433,7 +2434,6 @@ linearFunctionTestReplace(Loop *L, Basic
>>   // dynamically dead IV that wraps on the first loop iteration only, which is
>>   // not covered by the post-inc addrec. (If the new IV was not dynamically
>>   // dead, it could not be poison on the first iteration in the first place.)
>> -  Value *IncVar = IndVar->getIncomingValueForBlock(L->getLoopLatch());
>>   if (auto *BO = dyn_cast<BinaryOperator>(IncVar)) {
>>     const SCEVAddRecExpr *AR = cast<SCEVAddRecExpr>(SE->getSCEV(IncVar));
>>     if (BO->hasNoUnsignedWrap())
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190614/417a569e/attachment.html>