[llvm] r181230 - Rotate multi-exit loops even if the latch was simplified.
Andrew Trick
atrick at apple.com
Mon May 6 17:04:12 PDT 2013
On May 6, 2013, at 11:49 AM, Hal Finkel <hfinkel at anl.gov> wrote:
> ----- Original Message -----
>> Author: atrick
>> Date: Mon May 6 12:58:18 2013
>> New Revision: 181230
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=181230&view=rev
>> Log:
>> Rotate multi-exit loops even if the latch was simplified.
>>
>> Test case by Michele Scandale!
>>
>> Fixes PR10293: Load not hoisted out of loop with multiple exits.
>>
>> There are few regressions with this patch, now tracked by
>> rdar:13817079, and a roughly equal number of improvements. The
>> regressions are almost certainly back luck because LoopRotate has
>> very
>> little idea of whether rotation is profitable. Doing better requires
>> a
>> more comprehensive solution.
>>
>> This checkin is a quick fix that lacks generality (PR10293 has
>> a counter-example). But it trivially fixes the case in PR10293
>> without
>> interfering with other cases, and it does satify the criteria that
>> LoopRotate is a loop canonicalization pass that should avoid
>> heuristics and special cases.
>>
>> I can think of two approaches that would probably be better in
>> the long run. Ultimately they may both make sense.
>>
>> (1) LoopRotate should check that the current header would make a good
>> loop guard, and that the loop does not already has a sufficient
>> guard. The artifical SimplifiedLoopLatch check would be unnecessary,
>> and the design would be more general and canonical. Two difficulties:
>>
>> - We need a strong guarantee that we won't endlessly rotate, so the
>> analysis would need to be precise in order to avoid the
>> SimplifiedLoopLatch precondition.
>>
>> - Analysis like this are usually based on SCEV, which we don't want
>> to
>> rely on.
>
> Aside from the reliance on a loop guard that you're mentioned below, which sounds like something that we should fix regardless, why do you not want to rely on SCEV? My inclination would be to centralize loop analysis work in fewer, rather than more, places. As you know, there are a lot of corner cases ;)
>
> Also, what exactly do you mean by a guard condition. For example, SCEV can return (at least some) useful information about the loop in foo() in test-suite/SingleSource/UnitTests/2008-04-20-LoopBug2 -- and that loop does not end up with a guard.
Sure, SCEV can still do IV analysis. But without a loop guard, we end up with umax expressions for the trip count and exit values. In this example, LFTR is being defeated by the complex trip count expression. It seems like it may also be gross to have loop exit values expanded from umax instead of a simple phi/select. These are not horrible examples, but I'm not sure who else is relying on simple trip counts.
I think it makes sense to convert top-counted (while) loops into bottom counted (do) loops as a canonicalization. One approach I hacked up was to rotate if the header looked like a loop test, and the latch did not. It's not quite as simple as it sounds and I wasn't comfortable adding pattern matching to LoopRotate yet without considering the broader problem.
-Andy
>> (2) Rotate on-demand in late loop passes. This could even be done by
>> shoving the loop back on the queue after the optimization that needs
>> it. This could work well when we find LICM opportunities in
>> multi-branch loops. This requires some work, and it doesn't really
>> solve the problem of SCEV wanting a loop guard before the analysis.
>>
>> Modified:
>> llvm/trunk/lib/Transforms/Scalar/LoopRotation.cpp
>> llvm/trunk/test/Transforms/LoopRotate/simplifylatch.ll
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/LoopRotation.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopRotation.cpp?rev=181230&r1=181229&r2=181230&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/LoopRotation.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/LoopRotation.cpp Mon May 6
>> 12:58:18 2013
>> @@ -56,8 +56,8 @@ namespace {
>> }
>>
>> bool runOnLoop(Loop *L, LPPassManager &LPM);
>> - void simplifyLoopLatch(Loop *L);
>> - bool rotateLoop(Loop *L);
>> + bool simplifyLoopLatch(Loop *L);
>> + bool rotateLoop(Loop *L, bool SimplifiedLatch);
>>
>> private:
>> LoopInfo *LI;
>> @@ -84,13 +84,14 @@ bool LoopRotate::runOnLoop(Loop *L, LPPa
>> // Simplify the loop latch before attempting to rotate the header
>> // upward. Rotation may not be needed if the loop tail can be
>> folded into the
>> // loop exit.
>> - simplifyLoopLatch(L);
>> + bool SimplifiedLatch = simplifyLoopLatch(L);
>>
>> // One loop can be rotated multiple times.
>> bool MadeChange = false;
>> - while (rotateLoop(L))
>> + while (rotateLoop(L, SimplifiedLatch)) {
>> MadeChange = true;
>> -
>> + SimplifiedLatch = false;
>> + }
>> return MadeChange;
>> }
>>
>> @@ -212,25 +213,25 @@ static bool shouldSpeculateInstrs(BasicB
>> /// canonical form so downstream passes can handle it.
>> ///
>> /// I don't believe this invalidates SCEV.
>> -void LoopRotate::simplifyLoopLatch(Loop *L) {
>> +bool LoopRotate::simplifyLoopLatch(Loop *L) {
>> BasicBlock *Latch = L->getLoopLatch();
>> if (!Latch || Latch->hasAddressTaken())
>> - return;
>> + return false;
>>
>> BranchInst *Jmp = dyn_cast<BranchInst>(Latch->getTerminator());
>> if (!Jmp || !Jmp->isUnconditional())
>> - return;
>> + return false;
>>
>> BasicBlock *LastExit = Latch->getSinglePredecessor();
>> if (!LastExit || !L->isLoopExiting(LastExit))
>> - return;
>> + return false;
>>
>> BranchInst *BI = dyn_cast<BranchInst>(LastExit->getTerminator());
>> if (!BI)
>> - return;
>> + return false;
>>
>> if (!shouldSpeculateInstrs(Latch->begin(), Jmp))
>> - return;
>> + return false;
>>
>> DEBUG(dbgs() << "Folding loop latch " << Latch->getName() << "
>> into "
>> << LastExit->getName() << "\n");
>> @@ -253,10 +254,20 @@ void LoopRotate::simplifyLoopLatch(Loop
>> if (DominatorTree *DT = getAnalysisIfAvailable<DominatorTree>())
>> DT->eraseNode(Latch);
>> Latch->eraseFromParent();
>> + return true;
>> }
>>
>> /// Rotate loop LP. Return true if the loop is rotated.
>> -bool LoopRotate::rotateLoop(Loop *L) {
>> +///
>> +/// \param SimplifiedLatch is true if the latch was just folded into
>> the final
>> +/// loop exit. In this case we may want to rotate even though the
>> new latch is
>> +/// now an exiting branch. This rotation would have happened had the
>> latch not
>> +/// been simplified. However, if SimplifiedLatch is false, then we
>> avoid
>> +/// rotating loops in which the latch exits to avoid excessive or
>> endless
>> +/// rotation. LoopRotate should be repeatable and converge to a
>> canonical
>> +/// form. This property is satisfied because simplifying the loop
>> latch can only
>> +/// happen once across multiple invocations of the LoopRotate pass.
>> +bool LoopRotate::rotateLoop(Loop *L, bool SimplifiedLatch) {
>> // If the loop has only one block then there is not much to
>> rotate.
>> if (L->getBlocks().size() == 1)
>> return false;
>> @@ -276,7 +287,12 @@ bool LoopRotate::rotateLoop(Loop *L) {
>>
>> // If the loop latch already contains a branch that leaves the
>> loop then the
>> // loop is already rotated.
>> - if (OrigLatch == 0 || L->isLoopExiting(OrigLatch))
>> + if (OrigLatch == 0)
>> + return false;
>> +
>> + // Rotate if either the loop latch does *not* exit the loop, or if
>> the loop
>> + // latch was just simplified.
>> + if (L->isLoopExiting(OrigLatch) && !SimplifiedLatch)
>> return false;
>>
>> // Check size of original header and reject loop if it is very big
>> or we can't
>> @@ -505,4 +521,3 @@ bool LoopRotate::rotateLoop(Loop *L) {
>> ++NumRotated;
>> return true;
>> }
>> -
>>
>> Modified: llvm/trunk/test/Transforms/LoopRotate/simplifylatch.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopRotate/simplifylatch.ll?rev=181230&r1=181229&r2=181230&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/LoopRotate/simplifylatch.ll (original)
>> +++ llvm/trunk/test/Transforms/LoopRotate/simplifylatch.ll Mon May 6
>> 12:58:18 2013
>> @@ -1,4 +1,4 @@
>> -; RUN: opt -S < %s -loop-rotate -verify-dom-info -verify-loop-info |
>> FileCheck %s
>> +; RUN: opt -S < %s -loop-rotate -licm -verify-dom-info
>> -verify-loop-info | FileCheck %s
>> ; PR2624 unroll multiple exits
>>
>> @mode_table = global [4 x i32] zeroinitializer ; <[4 x i32]*>
>> [#uses=1]
>> @@ -37,3 +37,40 @@ bb5: ; preds = %bb2
>> declare i32 @fegetround()
>>
>> declare void @raise_exception() noreturn
>> +
>> +;CHECK: for.body.lr.ph:
>> +;CHECK-NEXT: %arrayidx1 = getelementptr inbounds i8* %CurPtr, i64 0
>> +;CHECK-NEXT: %0 = load i8* %arrayidx1, align 1
>> +;CHECK-NEXT: %conv2 = sext i8 %0 to i32
>> +;CHECK-NEXT: br label %for.body
>> +
>> +define i32 @foo(i8* %CurPtr, i32 %a) #0 {
>> +entry:
>> + br label %for.cond
>> +
>> +for.cond: ; preds = %for.inc, %entry
>> + %i.0 = phi i32 [ 1, %entry ], [ %inc, %for.inc ]
>> + %cmp = icmp ne i32 %i.0, %a
>> + br i1 %cmp, label %for.body, label %return
>> +
>> +for.body: ; preds = %for.cond
>> + %idxprom = zext i32 %i.0 to i64
>> + %arrayidx = getelementptr inbounds i8* %CurPtr, i64 %idxprom
>> + %0 = load i8* %arrayidx, align 1
>> + %conv = sext i8 %0 to i32
>> + %arrayidx1 = getelementptr inbounds i8* %CurPtr, i64 0
>> + %1 = load i8* %arrayidx1, align 1
>> + %conv2 = sext i8 %1 to i32
>> + %cmp3 = icmp ne i32 %conv, %conv2
>> + br i1 %cmp3, label %return, label %for.inc
>> +
>> +for.inc: ; preds = %for.body
>> + %inc = add i32 %i.0, 1
>> + br label %for.cond
>> +
>> +return: ; preds = %for.cond, %for.body
>> + %retval.0 = phi i32 [ 0, %for.body ], [ 1, %for.cond ]
>> + ret i32 %retval.0
>> +}
>> +
>> +attributes #0 = { nounwind uwtable }
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130506/b80bbb83/attachment.html>
More information about the llvm-commits
mailing list