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