From 6bc56d552ad52a19f23269e66fceea6c2beeb168 Mon Sep 17 00:00:00 2001 From: Michael Zolotukhin Date: Wed, 20 Jul 2016 01:55:27 +0000 Subject: [PATCH] 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. llvm-svn: 276077 --- llvm/lib/Transforms/Utils/LCSSA.cpp | 22 ++++- llvm/lib/Transforms/Utils/LoopSimplify.cpp | 62 +++++++++++--- llvm/test/Transforms/LCSSA/pr28424.ll | 87 ++++++++++++++++++++ llvm/test/Transforms/LCSSA/pr28608.ll | 35 ++++++++ llvm/test/Transforms/LoopSimplify/pr28272.ll | 76 +++++++++++++++++ 5 files changed, 267 insertions(+), 15 deletions(-) create mode 100644 llvm/test/Transforms/LCSSA/pr28424.ll create mode 100644 llvm/test/Transforms/LCSSA/pr28608.ll create mode 100644 llvm/test/Transforms/LoopSimplify/pr28272.ll diff --git a/llvm/lib/Transforms/Utils/LCSSA.cpp b/llvm/lib/Transforms/Utils/LCSSA.cpp index 9658966779b9..0d5a25b8ebc5 100644 --- a/llvm/lib/Transforms/Utils/LCSSA.cpp +++ b/llvm/lib/Transforms/Utils/LCSSA.cpp @@ -64,6 +64,7 @@ bool llvm::formLCSSAForInstructions(SmallVectorImpl &Worklist, DominatorTree &DT, LoopInfo &LI) { SmallVector UsesToRewrite; SmallVector ExitBlocks; + SmallSetVector PHIsToRemove; PredIteratorCache PredCache; bool Changed = false; @@ -115,7 +116,8 @@ bool llvm::formLCSSAForInstructions(SmallVectorImpl &Worklist, SmallVector AddedPHIs; SmallVector PostProcessPHIs; - SSAUpdater SSAUpdate; + SmallVector 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(SmallVectorImpl &Worklist, // 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(SmallVectorImpl &Worklist, 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; } diff --git a/llvm/lib/Transforms/Utils/LoopSimplify.cpp b/llvm/lib/Transforms/Utils/LoopSimplify.cpp index b3a928bf7753..2846e8f235b7 100644 --- a/llvm/lib/Transforms/Utils/LoopSimplify.cpp +++ b/llvm/lib/Transforms/Utils/LoopSimplify.cpp @@ -327,6 +327,8 @@ static Loop *separateNestedLoop(Loop *L, BasicBlock *Preheader, else NewOuter->addChildLoop(L->removeChildLoop(SubLoops.begin() + I)); + SmallVector 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, BasicBlock *Preheader, 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 ExitBlocks; + L->getExitBlocks(ExitBlocks); + SmallSetVector 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 WorklistSet; + for (BasicBlock *OuterBB: OuterLoopBlocks) { + for (Instruction &I : *OuterBB) { + for (Value *Op : I.operands()) { + Instruction *OpI = dyn_cast(Op); + if (!OpI || !L->contains(OpI)) + continue; + WorklistSet.insert(OpI); + } + } + } + SmallVector 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 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 diff --git a/llvm/test/Transforms/LCSSA/pr28424.ll b/llvm/test/Transforms/LCSSA/pr28424.ll new file mode 100644 index 000000000000..cd7969009fbb --- /dev/null +++ b/llvm/test/Transforms/LCSSA/pr28424.ll @@ -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 +} diff --git a/llvm/test/Transforms/LCSSA/pr28608.ll b/llvm/test/Transforms/LCSSA/pr28608.ll new file mode 100644 index 000000000000..3ba3fe8cda11 --- /dev/null +++ b/llvm/test/Transforms/LCSSA/pr28608.ll @@ -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 +} + diff --git a/llvm/test/Transforms/LoopSimplify/pr28272.ll b/llvm/test/Transforms/LoopSimplify/pr28272.ll new file mode 100644 index 000000000000..49990f92cabe --- /dev/null +++ b/llvm/test/Transforms/LoopSimplify/pr28272.ll @@ -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 = phi i32 [ undef, %loop1 ] + %b.ph = phi i32 [ 0, %loop1 ] + br label %loop2 + +loop2: + %a = phi i32 [ 0, %loop2.if.true ], [ %a, %loop2.if.false ], [ %a.ph, %loop2.preheader ], [0, %bb] + %b = phi i32 [ 1, %loop2.if.false ], [ %c, %loop2.if.true ], [ %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 +}