[PATCH] D63952: [LoopBase] Strengthen isLoopExiting by requiring that BB must be inside the loop.

[Florian Hahn via Phabricator via llvm-commits]

fhahn created this revision.
fhahn added reviewers: reames, efriedma, hfinkel.
Herald added subscribers: hiraditya, tpr, nhaehnle, jvesely, arsenm.
Herald added a project: LLVM.

Currently isLoopExiting returns true for BBs that are not part of the
loop. To avoid hiding subtle bugs, this patch adds an assertion to make
sure the passed BB is inside the loop. Alternatively, we could return
false for BBs outside the loop, but I think it makes sense to restrict
it to BBs inside a loop.

Just one use in AMDGPUTTIImpl needed updating.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63952

Files:
  llvm/include/llvm/Analysis/LoopInfo.h
  llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp


Index: llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp
@@ -117,8 +117,10 @@
       // Add a small bonus for each of such "if" statements.
       if (const BranchInst *Br = dyn_cast<BranchInst>(&I)) {
         if (UP.Threshold < MaxBoost && Br->isConditional()) {
-          if (L->isLoopExiting(Br->getSuccessor(0)) ||
-              L->isLoopExiting(Br->getSuccessor(1)))
+          BasicBlock *Succ0 = Br->getSuccessor(0);
+          BasicBlock *Succ1 = Br->getSuccessor(1);
+          if ((L->contains(Succ0) && L->isLoopExiting(Succ0)) ||
+              (L->contains(Succ1) && L->isLoopExiting(Succ1)))
             continue;
           if (dependsOnLocalPhi(L, Br->getCondition())) {
             UP.Threshold += UnrollThresholdIf;
Index: llvm/include/llvm/Analysis/LoopInfo.h
===================================================================
--- llvm/include/llvm/Analysis/LoopInfo.h
+++ llvm/include/llvm/Analysis/LoopInfo.h
@@ -201,9 +201,10 @@
   }
 
   /// True if terminator in the block can branch to another block that is
-  /// outside of the current loop.
+  /// outside of the current loop. \p BB must be inside the loop.
   bool isLoopExiting(const BlockT *BB) const {
     assert(!isInvalid() && "Loop not in a valid state!");
+    assert(contains(BB) && "Exiting block must be part of the loop");
     for (const auto &Succ : children<const BlockT *>(BB)) {
       if (!contains(Succ))
         return true;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D63952.207125.patch
Type: text/x-patch
Size: 1616 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190628/ff2abb29/attachment.bin>