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