[PATCH] D110492: [LoopInfo] Return conditional branch of preheader's predecessor if loop's exit has exactly two predecessors which are latch and guard

[Daniil Seredkin via Phabricator via llvm-commits]

vdsered created this revision.
vdsered added reviewers: Whitney, fhahn.
Herald added a subscriber: hiraditya.
vdsered requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

While working on an optimization for DSE I've noticed that for an example like the one below we return getLoopGuardBranch == nullptr and isGuarded == false for both loops even if they are in rotated form. But, technically, entry block decides if both loops execute. See IR in an example on godbolt <https://godbolt.org/z/xK6PWeEYq>.

  void func(int *P, int N) {
      for (int i = 0; i < N; ++i) {
          P[i] = 1;
      }
      for (int i = 0; i < N; ++i) {
          P[i] = 2;
      }
  }

This happens because it is not a canonical form (specifically, because of no dedicated exits). I suggest to relax this requirement a little and add a check for a case when there is an exit that has exactly two predecessors and both of them are latch from loop and the block whose branch instruction we return in getLoopGuardBranch. So, we find a guard in a case as in the example above.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110492

Files:
  llvm/lib/Analysis/LoopInfo.cpp
  llvm/unittests/Analysis/LoopInfoTest.cpp


Index: llvm/unittests/Analysis/LoopInfoTest.cpp
===================================================================
--- llvm/unittests/Analysis/LoopInfoTest.cpp
+++ llvm/unittests/Analysis/LoopInfoTest.cpp
@@ -1547,3 +1547,43 @@
     EXPECT_EQ(L->getLoopGuardBranch(), nullptr);
   });
 }
+
+TEST(LoopInfoTest, NoDedicatedExitsButTheSameBBWithGuard) {
+  const char *ModuleStr =
+      "target datalayout = \"e-m:o-i64:64-f80:128-n8:16:32:64-S128\"\n"
+      "define void @foo(i32* %B, i64 signext %nx, i1 %cond) {\n"
+      "entry:\n"
+      "  %cmp.guard = icmp slt i64 0, %nx\n"
+      "  br i1 %cmp.guard, label %for.i.preheader, label %for.i.exit\n"
+      "for.i.preheader:\n"
+      "  br label %for.i\n"
+      "for.i:\n"
+      "  %i = phi i64 [ 0, %for.i.preheader ], [ %inc13, %for.i ]\n"
+      "  %Bi = getelementptr inbounds i32, i32* %B, i64 %i\n"
+      "  store i32 0, i32* %Bi, align 4\n"
+      "  %inc13 = add nsw i64 %i, 1\n"
+      "  %cmp = icmp slt i64 %inc13, %nx\n"
+      "  br i1 %cmp, label %for.i, label %for.i.exit\n"
+      "for.i.exit:\n"
+      "  ret void\n"
+      "}\n";
+
+  // Parse the module.
+  LLVMContext Context;
+  std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleStr);
+
+  runWithLoopInfo(*M, "foo", [&](Function &F, LoopInfo &LI) {
+    Function::iterator FI = F.begin();
+    assert(FI->getName() == "entry");
+
+    FI = ++FI;
+    BasicBlock *Header = &*(++FI);
+    assert(Header->getName() == "for.i");
+
+    Loop *L = LI.getLoopFor(Header);
+    EXPECT_NE(L, nullptr);
+
+    EXPECT_EQ(L->isGuarded(), true);
+    EXPECT_NE(L->getLoopGuardBranch(), nullptr);
+  });
+}
Index: llvm/lib/Analysis/LoopInfo.cpp
===================================================================
--- llvm/lib/Analysis/LoopInfo.cpp
+++ llvm/lib/Analysis/LoopInfo.cpp
@@ -364,17 +364,14 @@
 }
 
 BranchInst *Loop::getLoopGuardBranch() const {
-  if (!isLoopSimplifyForm())
-    return nullptr;
-
   BasicBlock *Preheader = getLoopPreheader();
-  assert(Preheader && getLoopLatch() &&
-         "Expecting a loop with valid preheader and latch");
-
+  if (!Preheader)
+    return nullptr;
+  if (!getLoopLatch())
+    return nullptr;
   // Loop should be in rotate form.
   if (!isRotatedForm())
     return nullptr;
-
   // Disallow loops with more than one unique exit block, as we do not verify
   // that GuardOtherSucc post dominates all exit blocks.
   BasicBlock *ExitFromLatch = getUniqueExitBlock();
@@ -384,7 +381,6 @@
   BasicBlock *GuardBB = Preheader->getUniquePredecessor();
   if (!GuardBB)
     return nullptr;
-
   assert(GuardBB->getTerminator() && "Expecting valid guard terminator");
 
   BranchInst *GuardBI = dyn_cast<BranchInst>(GuardBB->getTerminator());
@@ -394,6 +390,12 @@
   BasicBlock *GuardOtherSucc = (GuardBI->getSuccessor(0) == Preheader)
                                    ? GuardBI->getSuccessor(1)
                                    : GuardBI->getSuccessor(0);
+  if (!hasDedicatedExits()) {
+    if (ExitFromLatch->hasNPredecessors(2) && ExitFromLatch == GuardOtherSucc) {
+      return GuardBI;
+    }
+    return nullptr;
+  }
 
   // Check if ExitFromLatch (or any BasicBlock which is an empty unique
   // successor of ExitFromLatch) is equal to GuardOtherSucc. If


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D110492.375082.patch
Type: text/x-patch
Size: 3249 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210926/6dbc9e48/attachment.bin>