<div dir="ltr">We branched at r275826 which led to us missing out on this for the 3.9 branch. Can we merge this in?<br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jul 19, 2016 at 6:55 PM, Michael Zolotukhin via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Author: mzolotukhin<br>
Date: Tue Jul 19 20:55:27 2016<br>
New Revision: 276077<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=276077&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=276077&view=rev</a><br>
Log:<br>
Revert "Revert r275883 and r275891. They seem to cause PR28608."<br>
<br>
This reverts commit r276064, and thus reapplies r275891 and r275883 with<br>
a fix for PR28608.<br>
<br>
Added:<br>
llvm/trunk/test/Transforms/LCSSA/pr28424.ll<br>
llvm/trunk/test/Transforms/LCSSA/pr28608.ll<br>
llvm/trunk/test/Transforms/LoopSimplify/pr28272.ll<br>
Modified:<br>
llvm/trunk/lib/Transforms/Utils/LCSSA.cpp<br>
llvm/trunk/lib/Transforms/Utils/LoopSimplify.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/Utils/LCSSA.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/LCSSA.cpp?rev=276077&r1=276076&r2=276077&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/LCSSA.cpp?rev=276077&r1=276076&r2=276077&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Utils/LCSSA.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Utils/LCSSA.cpp Tue Jul 19 20:55:27 2016<br>
@@ -64,6 +64,7 @@ bool llvm::formLCSSAForInstructions(Smal<br>
DominatorTree &DT, LoopInfo &LI) {<br>
SmallVector<Use *, 16> UsesToRewrite;<br>
SmallVector<BasicBlock *, 8> ExitBlocks;<br>
+ SmallSetVector<PHINode *, 16> PHIsToRemove;<br>
PredIteratorCache PredCache;<br>
bool Changed = false;<br>
<br>
@@ -115,7 +116,8 @@ bool llvm::formLCSSAForInstructions(Smal<br>
SmallVector<PHINode *, 16> AddedPHIs;<br>
SmallVector<PHINode *, 8> PostProcessPHIs;<br>
<br>
- SSAUpdater SSAUpdate;<br>
+ SmallVector<PHINode *, 4> InsertedPHIs;<br>
+ SSAUpdater SSAUpdate(&InsertedPHIs);<br>
SSAUpdate.Initialize(I->getType(), I->getName());<br>
<br>
// Insert the LCSSA phi's into all of the exit blocks dominated by the<br>
@@ -184,6 +186,14 @@ bool llvm::formLCSSAForInstructions(Smal<br>
<br>
// Otherwise, do full PHI insertion.<br>
SSAUpdate.RewriteUse(*UseToRewrite);<br>
+<br>
+ // SSAUpdater might have inserted phi-nodes inside other loops. We'll need<br>
+ // to post-process them to keep LCSSA form.<br>
+ for (PHINode *InsertedPN : InsertedPHIs) {<br>
+ if (auto *OtherLoop = LI.getLoopFor(InsertedPN->getParent()))<br>
+ if (!L->contains(OtherLoop))<br>
+ PostProcessPHIs.push_back(InsertedPN);<br>
+ }<br>
}<br>
<br>
// Post process PHI instructions that were inserted into another disjoint<br>
@@ -196,13 +206,19 @@ bool llvm::formLCSSAForInstructions(Smal<br>
Worklist.push_back(PostProcessPN);<br>
}<br>
<br>
- // Remove PHI nodes that did not have any uses rewritten.<br>
+ // Keep track of PHI nodes that we want to remove because they did not have<br>
+ // any uses rewritten.<br>
for (PHINode *PN : AddedPHIs)<br>
if (PN->use_empty())<br>
- PN->eraseFromParent();<br>
+ PHIsToRemove.insert(PN);<br>
<br>
Changed = true;<br>
}<br>
+ // Remove PHI nodes that did not have any uses rewritten.<br>
+ for (PHINode *PN : PHIsToRemove) {<br>
+ assert (PN->use_empty() && "Trying to remove a phi with uses.");<br>
+ PN->eraseFromParent();<br>
+ }<br>
return Changed;<br>
}<br>
<br>
<br>
Modified: llvm/trunk/lib/Transforms/Utils/LoopSimplify.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/LoopSimplify.cpp?rev=276077&r1=276076&r2=276077&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/LoopSimplify.cpp?rev=276077&r1=276076&r2=276077&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Utils/LoopSimplify.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Utils/LoopSimplify.cpp Tue Jul 19 20:55:27 2016<br>
@@ -327,6 +327,8 @@ static Loop *separateNestedLoop(Loop *L,<br>
else<br>
NewOuter->addChildLoop(L->removeChildLoop(SubLoops.begin() + I));<br>
<br>
+ SmallVector<BasicBlock *, 8> OuterLoopBlocks;<br>
+ OuterLoopBlocks.push_back(NewBB);<br>
// Now that we know which blocks are in L and which need to be moved to<br>
// OuterLoop, move any blocks that need it.<br>
for (unsigned i = 0; i != L->getBlocks().size(); ++i) {<br>
@@ -334,12 +336,53 @@ static Loop *separateNestedLoop(Loop *L,<br>
if (!BlocksInL.count(BB)) {<br>
// Move this block to the parent, updating the exit blocks sets<br>
L->removeBlockFromLoop(BB);<br>
- if ((*LI)[BB] == L)<br>
+ if ((*LI)[BB] == L) {<br>
LI->changeLoopFor(BB, NewOuter);<br>
+ OuterLoopBlocks.push_back(BB);<br>
+ }<br>
--i;<br>
}<br>
}<br>
<br>
+ // Split edges to exit blocks from the inner loop, if they emerged in the<br>
+ // process of separating the outer one.<br>
+ SmallVector<BasicBlock *, 8> ExitBlocks;<br>
+ L->getExitBlocks(ExitBlocks);<br>
+ SmallSetVector<BasicBlock *, 8> ExitBlockSet(ExitBlocks.begin(),<br>
+ ExitBlocks.end());<br>
+ for (BasicBlock *ExitBlock : ExitBlockSet) {<br>
+ if (any_of(predecessors(ExitBlock),<br>
+ [L](BasicBlock *BB) { return !L->contains(BB); })) {<br>
+ rewriteLoopExitBlock(L, ExitBlock, DT, LI, PreserveLCSSA);<br>
+ }<br>
+ }<br>
+<br>
+ if (PreserveLCSSA) {<br>
+ // Fix LCSSA form for L. Some values, which previously were only used inside<br>
+ // L, can now be used in NewOuter loop. We need to insert phi-nodes for them<br>
+ // in corresponding exit blocks.<br>
+<br>
+ // Go through all instructions in OuterLoopBlocks and check if they are<br>
+ // using operands from the inner loop. In this case we'll need to fix LCSSA<br>
+ // for these instructions.<br>
+ SmallSetVector<Instruction *, 8> WorklistSet;<br>
+ for (BasicBlock *OuterBB: OuterLoopBlocks) {<br>
+ for (Instruction &I : *OuterBB) {<br>
+ for (Value *Op : I.operands()) {<br>
+ Instruction *OpI = dyn_cast<Instruction>(Op);<br>
+ if (!OpI || !L->contains(OpI))<br>
+ continue;<br>
+ WorklistSet.insert(OpI);<br>
+ }<br>
+ }<br>
+ }<br>
+ SmallVector<Instruction *, 8> Worklist(WorklistSet.begin(),<br>
+ WorklistSet.end());<br>
+ formLCSSAForInstructions(Worklist, *DT, *LI);<br>
+ assert(NewOuter->isRecursivelyLCSSAForm(*DT) &&<br>
+ "LCSSA is broken after separating nested loops!");<br>
+ }<br>
+<br>
return NewOuter;<br>
}<br>
<br>
@@ -541,17 +584,12 @@ ReprocessLoop:<br>
SmallSetVector<BasicBlock *, 8> ExitBlockSet(ExitBlocks.begin(),<br>
ExitBlocks.end());<br>
for (BasicBlock *ExitBlock : ExitBlockSet) {<br>
- for (pred_iterator PI = pred_begin(ExitBlock), PE = pred_end(ExitBlock);<br>
- PI != PE; ++PI)<br>
- // Must be exactly this loop: no subloops, parent loops, or non-loop preds<br>
- // allowed.<br>
- if (!L->contains(*PI)) {<br>
- if (rewriteLoopExitBlock(L, ExitBlock, DT, LI, PreserveLCSSA)) {<br>
- ++NumInserted;<br>
- Changed = true;<br>
- }<br>
- break;<br>
- }<br>
+ if (any_of(predecessors(ExitBlock),<br>
+ [L](BasicBlock *BB) { return !L->contains(BB); })) {<br>
+ rewriteLoopExitBlock(L, ExitBlock, DT, LI, PreserveLCSSA);<br>
+ ++NumInserted;<br>
+ Changed = true;<br>
+ }<br>
}<br>
<br>
// If the header has more than two predecessors at this point (from the<br>
<br>
Added: llvm/trunk/test/Transforms/LCSSA/pr28424.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LCSSA/pr28424.ll?rev=276077&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LCSSA/pr28424.ll?rev=276077&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/LCSSA/pr28424.ll (added)<br>
+++ llvm/trunk/test/Transforms/LCSSA/pr28424.ll Tue Jul 19 20:55:27 2016<br>
@@ -0,0 +1,87 @@<br>
+; RUN: opt < %s -lcssa -S -o - | FileCheck %s<br>
+target triple = "x86_64-unknown-linux-gnu"<br>
+<br>
+; PR28424<br>
+; Here LCSSA adds phi-nodes for %x into the loop exits. Then, SSAUpdater needs<br>
+; to insert phi-nodes to merge these values. That creates a new def, which in<br>
+; its turn needs another LCCSA phi-node, and this test ensures that we insert<br>
+; it.<br>
+<br>
+; CHECK-LABEL: @foo1<br>
+define internal i32 @foo1() {<br>
+entry:<br>
+ br label %header<br>
+<br>
+header:<br>
+ %x = add i32 0, 1<br>
+ br i1 undef, label %if, label %loopexit1<br>
+<br>
+if:<br>
+ br i1 undef, label %latch, label %loopexit2<br>
+<br>
+latch:<br>
+ br i1 undef, label %header, label %loopexit3<br>
+<br>
+; CHECK: loopexit1:<br>
+; CHECK: %x.lcssa = phi i32 [ %x, %header ]<br>
+loopexit1:<br>
+ br label %loop_with_insert_point<br>
+<br>
+; CHECK: loopexit2:<br>
+; CHECK: %x.lcssa1 = phi i32 [ %x, %if ]<br>
+loopexit2:<br>
+ br label %exit<br>
+<br>
+; CHECK: loopexit3:<br>
+; CHECK: %x.lcssa2 = phi i32 [ %x, %latch ]<br>
+loopexit3:<br>
+ br label %loop_with_insert_point<br>
+<br>
+; CHECK: loop_with_insert_point:<br>
+; CHECK: %x4 = phi i32 [ %x4, %loop_with_insert_point ], [ %x.lcssa2, %loopexit3 ], [ %x.lcssa, %loopexit1 ]<br>
+loop_with_insert_point:<br>
+ br i1 undef, label %loop_with_insert_point, label %bb<br>
+<br>
+; CHECK: bb:<br>
+; CHECK: %x4.lcssa = phi i32 [ %x4, %loop_with_insert_point ]<br>
+bb:<br>
+ br label %exit<br>
+<br>
+; CHECK: exit:<br>
+; CHECK: %x3 = phi i32 [ %x4.lcssa, %bb ], [ %x.lcssa1, %loopexit2 ]<br>
+exit:<br>
+ ret i32 %x<br>
+}<br>
+<br>
+; CHECK-LABEL: @foo2<br>
+define internal i32 @foo2() {<br>
+entry:<br>
+ br label %header<br>
+<br>
+header:<br>
+ %x = add i32 0, 1<br>
+ br i1 undef, label %latch, label %loopexit1<br>
+<br>
+latch:<br>
+ br i1 undef, label %header, label %loopexit2<br>
+<br>
+; CHECK: loopexit1:<br>
+; CHECK: %x.lcssa = phi i32 [ %x, %header ]<br>
+loopexit1:<br>
+ br label %loop_with_insert_point<br>
+<br>
+; CHECK: loopexit2:<br>
+; CHECK: %x.lcssa1 = phi i32 [ %x, %latch ]<br>
+loopexit2:<br>
+ br label %loop_with_insert_point<br>
+<br>
+; CHECK: loop_with_insert_point:<br>
+; CHECK: %x2 = phi i32 [ %x2, %loop_with_insert_point ], [ %x.lcssa1, %loopexit2 ], [ %x.lcssa, %loopexit1 ]<br>
+loop_with_insert_point:<br>
+ br i1 undef, label %loop_with_insert_point, label %exit<br>
+<br>
+; CHECK: exit:<br>
+; CHECK: %x2.lcssa = phi i32 [ %x2, %loop_with_insert_point ]<br>
+exit:<br>
+ ret i32 %x<br>
+}<br>
<br>
Added: llvm/trunk/test/Transforms/LCSSA/pr28608.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LCSSA/pr28608.ll?rev=276077&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LCSSA/pr28608.ll?rev=276077&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/LCSSA/pr28608.ll (added)<br>
+++ llvm/trunk/test/Transforms/LCSSA/pr28608.ll Tue Jul 19 20:55:27 2016<br>
@@ -0,0 +1,35 @@<br>
+; RUN: opt < %s -lcssa -disable-output<br>
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"<br>
+target triple = "x86_64-unknown-linux-gnu"<br>
+<br>
+; PR28608<br>
+; Check that we don't crash on this test.<br>
+<br>
+define void @foo() {<br>
+entry:<br>
+ br label %bb1<br>
+<br>
+bb1:<br>
+ br label %bb2<br>
+<br>
+bb2:<br>
+ %x = phi i32 [ undef, %bb5 ], [ undef, %bb1 ]<br>
+ br i1 undef, label %bb3, label %bb6<br>
+<br>
+bb3:<br>
+ br i1 undef, label %bb5, label %bb4<br>
+<br>
+bb4:<br>
+ br label %bb6<br>
+<br>
+bb5:<br>
+ br label %bb2<br>
+<br>
+bb6:<br>
+ br label %bb1<br>
+<br>
+exit:<br>
+ %y = add i32 0, %x<br>
+ ret void<br>
+}<br>
+<br>
<br>
Added: llvm/trunk/test/Transforms/LoopSimplify/pr28272.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopSimplify/pr28272.ll?rev=276077&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopSimplify/pr28272.ll?rev=276077&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/LoopSimplify/pr28272.ll (added)<br>
+++ llvm/trunk/test/Transforms/LoopSimplify/pr28272.ll Tue Jul 19 20:55:27 2016<br>
@@ -0,0 +1,76 @@<br>
+; RUN: opt < %s -lcssa -loop-unroll -S | FileCheck %s<br>
+target triple = "x86_64-unknown-linux-gnu"<br>
+<br>
+; PR28272<br>
+; When LoopSimplify separates nested loops, it might break LCSSA form: values<br>
+; from the original loop might be used in the outer loop. This test invokes<br>
+; loop-unroll, which calls loop-simplify before itself. If LCSSA is broken<br>
+; after loop-simplify, we crash on assertion.<br>
+<br>
+; CHECK-LABEL: @foo<br>
+define void @foo() {<br>
+entry:<br>
+ br label %header<br>
+<br>
+header:<br>
+ br label %loop1<br>
+<br>
+loop1:<br>
+ br i1 true, label %loop1, label %bb43<br>
+<br>
+bb43:<br>
+ %a = phi i32 [ undef, %loop1 ], [ 0, %bb45 ], [ %a, %bb54 ]<br>
+ %b = phi i32 [ 0, %loop1 ], [ 1, %bb54 ], [ %c, %bb45 ]<br>
+ br i1 true, label %bb114, label %header<br>
+<br>
+bb114:<br>
+ %c = add i32 0, 1<br>
+ %d = add i32 0, 1<br>
+ br i1 true, label %bb45, label %bb54<br>
+<br>
+bb45:<br>
+ %x = add i32 %d, 0<br>
+ br label %bb43<br>
+<br>
+bb54:<br>
+ br label %bb43<br>
+}<br>
+<br>
+; CHECK-LABEL: @foo2<br>
+define void @foo2() {<br>
+entry:<br>
+ br label %outer<br>
+<br>
+outer.loopexit:<br>
+ br label %outer<br>
+<br>
+outer:<br>
+ br label %loop1<br>
+<br>
+loop1:<br>
+ br i1 true, label %loop1, label %loop2.preheader<br>
+<br>
+loop2.preheader:<br>
+ %<a href="http://a.ph" rel="noreferrer" target="_blank">a.ph</a> = phi i32 [ undef, %loop1 ]<br>
+ %<a href="http://b.ph" rel="noreferrer" target="_blank">b.ph</a> = phi i32 [ 0, %loop1 ]<br>
+ br label %loop2<br>
+<br>
+loop2:<br>
+ %a = phi i32 [ 0, %loop2.if.true ], [ %a, %loop2.if.false ], [ %<a href="http://a.ph" rel="noreferrer" target="_blank">a.ph</a>, %loop2.preheader ], [0, %bb]<br>
+ %b = phi i32 [ 1, %loop2.if.false ], [ %c, %loop2.if.true ], [ %<a href="http://b.ph" rel="noreferrer" target="_blank">b.ph</a>, %loop2.preheader ], [%c, %bb]<br>
+ br i1 true, label %loop2.if, label %outer.loopexit<br>
+<br>
+loop2.if:<br>
+ %c = add i32 0, 1<br>
+ switch i32 undef, label %loop2.if.false [i32 0, label %loop2.if.true<br>
+ i32 1, label %bb]<br>
+<br>
+loop2.if.true:<br>
+ br i1 undef, label %loop2, label %bb<br>
+<br>
+loop2.if.false:<br>
+ br label %loop2<br>
+<br>
+bb:<br>
+ br label %loop2<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>