From: Whitney Tsang Date: Thu, 26 Sep 2019 20:20:42 +0000 (+0000) Subject: [LOOPGUARD] Disable loop with multiple loop exiting blocks. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=49f2855c39891aac8141c829b6b9069e0355a6f8;p=llvm [LOOPGUARD] Disable loop with multiple loop exiting blocks. Summary: As discussed in the loop group meeting. With the current definition of loop guard, we should not allow multiple loop exiting blocks. For loops that has multiple loop exiting blocks, we can simply unable to find the loop guard. When getUniqueExitBlock() obtains a vector size not equals to one, that means there is either no exit blocks or there exists more than one unique block the loop exit to. If we don't disallow loop with multiple loop exit blocks, then with our current implementation, there can exist exit blocks don't post dominated by the non pre-header successor of the guard block. Reviewer: reames, Meinersbur, kbarton, etiotto, bmahjour Reviewed By: Meinersbur, kbarton Subscribers: fhahn, hiraditya, llvm-commits Tag: LLVM Differential Revision: https://reviews.llvm.org/D66529 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@373011 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Analysis/LoopInfo.cpp b/lib/Analysis/LoopInfo.cpp index 5d481a4131a..c37a5c48e50 100644 --- a/lib/Analysis/LoopInfo.cpp +++ b/lib/Analysis/LoopInfo.cpp @@ -369,18 +369,16 @@ bool Loop::isAuxiliaryInductionVariable(PHINode &AuxIndVar, BranchInst *Loop::getLoopGuardBranch() const { assert(isLoopSimplifyForm() && "Only valid for loop in simplify form"); BasicBlock *Preheader = getLoopPreheader(); - BasicBlock *Latch = getLoopLatch(); - assert(Preheader && Latch && + assert(Preheader && getLoopLatch() && "Expecting a loop with valid preheader and latch"); - assert(isLoopExiting(Latch) && "Only valid for rotated loop"); + assert(isLoopExiting(getLoopLatch()) && "Only valid for rotated loop"); - Instruction *LatchTI = Latch->getTerminator(); - if (!LatchTI || LatchTI->getNumSuccessors() != 2) + // Disallow loops with more than one unique exit block, as we do not verify + // that GuardOtherSucc post dominates all exit blocks. + BasicBlock *ExitFromLatch = getUniqueExitBlock(); + if (!ExitFromLatch) return nullptr; - BasicBlock *ExitFromLatch = (LatchTI->getSuccessor(0) == getHeader()) - ? LatchTI->getSuccessor(1) - : LatchTI->getSuccessor(0); BasicBlock *ExitFromLatchSucc = ExitFromLatch->getUniqueSuccessor(); if (!ExitFromLatchSucc) return nullptr; diff --git a/unittests/Analysis/LoopInfoTest.cpp b/unittests/Analysis/LoopInfoTest.cpp index 75e74295033..2528078ac67 100644 --- a/unittests/Analysis/LoopInfoTest.cpp +++ b/unittests/Analysis/LoopInfoTest.cpp @@ -868,6 +868,126 @@ TEST(LoopInfoTest, ZextIndVar) { }); } +TEST(LoopInfoTest, MultiExitingLoop) { + const char *ModuleStr = + "define void @foo(i32* %A, i32 %ub, i1 %cond) {\n" + "entry:\n" + " %guardcmp = icmp slt i32 0, %ub\n" + " br i1 %guardcmp, label %for.preheader, label %for.end\n" + "for.preheader:\n" + " br label %for.body\n" + "for.body:\n" + " %i = phi i32 [ 0, %for.preheader ], [ %inc, %for.body.1 ]\n" + " br i1 %cond, label %for.body.1, label %for.exit\n" + "for.body.1:\n" + " %idxprom = sext i32 %i to i64\n" + " %arrayidx = getelementptr inbounds i32, i32* %A, i64 %idxprom\n" + " store i32 %i, i32* %arrayidx, align 4\n" + " %inc = add nsw i32 %i, 1\n" + " %cmp = icmp slt i32 %inc, %ub\n" + " br i1 %cmp, label %for.body, label %for.exit\n" + "for.exit:\n" + " br label %for.end\n" + "for.end:\n" + " ret void\n" + "}\n"; + + // Parse the module. + LLVMContext Context; + std::unique_ptr M = makeLLVMModule(Context, ModuleStr); + + runWithLoopInfoPlus( + *M, "foo", + [&](Function &F, LoopInfo &LI, ScalarEvolution &SE) { + Function::iterator FI = F.begin(); + BasicBlock *Entry = &*(FI); + BranchInst *Guard = dyn_cast(Entry->getTerminator()); + // First two basic block are entry and for.preheader - skip them. + ++FI; + BasicBlock *Header = &*(++FI); + assert(Header->getName() == "for.body"); + Loop *L = LI.getLoopFor(Header); + EXPECT_NE(L, nullptr); + + Optional Bounds = L->getBounds(SE); + EXPECT_NE(Bounds, None); + ConstantInt *InitialIVValue = + dyn_cast(&Bounds->getInitialIVValue()); + EXPECT_TRUE(InitialIVValue && InitialIVValue->isZero()); + EXPECT_EQ(Bounds->getStepInst().getName(), "inc"); + ConstantInt *StepValue = + dyn_cast_or_null(Bounds->getStepValue()); + EXPECT_TRUE(StepValue && StepValue->isOne()); + EXPECT_EQ(Bounds->getFinalIVValue().getName(), "ub"); + EXPECT_EQ(Bounds->getCanonicalPredicate(), ICmpInst::ICMP_SLT); + EXPECT_EQ(Bounds->getDirection(), + Loop::LoopBounds::Direction::Increasing); + EXPECT_EQ(L->getInductionVariable(SE)->getName(), "i"); + EXPECT_EQ(L->getLoopGuardBranch(), Guard); + EXPECT_TRUE(L->isGuarded()); + }); +} + +TEST(LoopInfoTest, MultiExitLoop) { + const char *ModuleStr = + "define void @foo(i32* %A, i32 %ub, i1 %cond) {\n" + "entry:\n" + " %guardcmp = icmp slt i32 0, %ub\n" + " br i1 %guardcmp, label %for.preheader, label %for.end\n" + "for.preheader:\n" + " br label %for.body\n" + "for.body:\n" + " %i = phi i32 [ 0, %for.preheader ], [ %inc, %for.body.1 ]\n" + " br i1 %cond, label %for.body.1, label %for.exit\n" + "for.body.1:\n" + " %idxprom = sext i32 %i to i64\n" + " %arrayidx = getelementptr inbounds i32, i32* %A, i64 %idxprom\n" + " store i32 %i, i32* %arrayidx, align 4\n" + " %inc = add nsw i32 %i, 1\n" + " %cmp = icmp slt i32 %inc, %ub\n" + " br i1 %cmp, label %for.body, label %for.exit.1\n" + "for.exit:\n" + " br label %for.end\n" + "for.exit.1:\n" + " br label %for.end\n" + "for.end:\n" + " ret void\n" + "}\n"; + + // Parse the module. + LLVMContext Context; + std::unique_ptr M = makeLLVMModule(Context, ModuleStr); + + runWithLoopInfoPlus( + *M, "foo", + [&](Function &F, LoopInfo &LI, ScalarEvolution &SE) { + Function::iterator FI = F.begin(); + // First two basic block are entry and for.preheader - skip them. + ++FI; + BasicBlock *Header = &*(++FI); + assert(Header->getName() == "for.body"); + Loop *L = LI.getLoopFor(Header); + EXPECT_NE(L, nullptr); + + Optional Bounds = L->getBounds(SE); + EXPECT_NE(Bounds, None); + ConstantInt *InitialIVValue = + dyn_cast(&Bounds->getInitialIVValue()); + EXPECT_TRUE(InitialIVValue && InitialIVValue->isZero()); + EXPECT_EQ(Bounds->getStepInst().getName(), "inc"); + ConstantInt *StepValue = + dyn_cast_or_null(Bounds->getStepValue()); + EXPECT_TRUE(StepValue && StepValue->isOne()); + EXPECT_EQ(Bounds->getFinalIVValue().getName(), "ub"); + EXPECT_EQ(Bounds->getCanonicalPredicate(), ICmpInst::ICMP_SLT); + EXPECT_EQ(Bounds->getDirection(), + Loop::LoopBounds::Direction::Increasing); + EXPECT_EQ(L->getInductionVariable(SE)->getName(), "i"); + EXPECT_EQ(L->getLoopGuardBranch(), nullptr); + EXPECT_FALSE(L->isGuarded()); + }); +} + TEST(LoopInfoTest, UnguardedLoop) { const char *ModuleStr = "define void @foo(i32* %A, i32 %ub) {\n"