[PATCH] D96706: [llvm] Bug fix: Loop interchange for multiple loops with conditional statements

[Masakazu Ueno via Phabricator via llvm-commits]

masakazu.ueno created this revision.
masakazu.ueno added reviewers: fhahn, hfinkel, qcolombet.
masakazu.ueno added a project: LLVM.
Herald added a subscriber: hiraditya.
masakazu.ueno requested review of this revision.
Herald added a subscriber: llvm-commits.

Loop interchange for a multiple loop containing if-statement may be performed incorrectly.
Therefore, I modified the loop interchange so that it does not optimize loops that contain conditional statements.

For example, in the following example, "-1, -1" is correct. However, if the loop is interchanged, "1, 1" is output.

  #include <stdio.h>
  
  void bar(int x[restrict 2], int a[restrict 2][2]) {
      for (int i=0; i<2; ++i) {
          for (int j=0; j<2; ++j) {
              for (int k=0; k<2; ++k) {
                  if (x[j] == a[k][i]) {
                      x[j] = i - k;
                  }
              }
          }
      }
  }
  
  int main(void) {
      int x[2] = {2, 2};
      int a[2][2] = {{0, 2}, {2, 0}};
      bar(x, a);
      printf("%d, %d\n", x[0], x[1]);
      return 0;
  }

Specified options are "-Ofast -mllvm -enable-loopinterchange=true -flegacy-pass-manager".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96706

Files:
  llvm/lib/Transforms/Scalar/LoopInterchange.cpp


Index: llvm/lib/Transforms/Scalar/LoopInterchange.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/LoopInterchange.cpp
+++ llvm/lib/Transforms/Scalar/LoopInterchange.cpp
@@ -328,8 +328,8 @@
 class LoopInterchangeLegality {
 public:
   LoopInterchangeLegality(Loop *Outer, Loop *Inner, ScalarEvolution *SE,
-                          OptimizationRemarkEmitter *ORE)
-      : OuterLoop(Outer), InnerLoop(Inner), SE(SE), ORE(ORE) {}
+                          OptimizationRemarkEmitter *ORE, DominatorTree *DT)
+     : OuterLoop(Outer), InnerLoop(Inner), SE(SE), ORE(ORE), DT(DT) {}
 
   /// Check if the loops can be interchanged.
   bool canInterchangeLoops(unsigned InnerLoopId, unsigned OuterLoopId,
@@ -347,6 +347,7 @@
 
 private:
   bool tightlyNested(Loop *Outer, Loop *Inner);
+  bool hasConditionalStatements(Loop *L);
   bool containsUnsafeInstructions(BasicBlock *BB);
 
   /// Discover induction and reduction PHIs in the header of \p L. Induction
@@ -361,6 +362,7 @@
   Loop *InnerLoop;
 
   ScalarEvolution *SE;
+  DominatorTree *DT;
 
   /// Interface to emit optimization remarks.
   OptimizationRemarkEmitter *ORE;
@@ -542,7 +544,7 @@
     Loop *InnerLoop = LoopList[InnerLoopId];
     Loop *OuterLoop = LoopList[OuterLoopId];
 
-    LoopInterchangeLegality LIL(OuterLoop, InnerLoop, SE, ORE);
+    LoopInterchangeLegality LIL(OuterLoop, InnerLoop, SE, ORE, DT);
     if (!LIL.canInterchangeLoops(InnerLoopId, OuterLoopId, DependencyMatrix)) {
       LLVM_DEBUG(dbgs() << "Not interchanging loops. Cannot prove legality.\n");
       return false;
@@ -679,6 +681,21 @@
   return nullptr;
 }
 
+/// Return true if the Loop has conditional statements.
+bool LoopInterchangeLegality::hasConditionalStatements (Loop *L) {
+  SmallVector<BasicBlock *, 4> ExitingBlocks;
+  L->getExitingBlocks(ExitingBlocks);
+
+  for (const auto BB : L->blocks()) {
+    for (BasicBlock *ExitingBlock : ExitingBlocks) {
+      if (!DT->dominates(BB, ExitingBlock)) {
+       return true;
+      }
+    }
+  }
+  return false;
+}
+
 bool LoopInterchangeLegality::findInductionAndReductions(
     Loop *L, SmallVector<PHINode *, 8> &Inductions, Loop *InnerLoop) {
   if (!L->getLoopLatch() || !L->getLoopPredecessor())
@@ -760,6 +777,20 @@
     return true;
   }
 
+  if (hasConditionalStatements(InnerLoop)) {
+    LLVM_DEBUG(
+              dbgs() << "Loops including conditional statements cannot be"
+                     << " interchanged.\n ");
+    ORE->emit([&]() {
+      return OptimizationRemarkMissed(DEBUG_TYPE,
+                                     "UnsupportedConditionalStatements",
+                                      InnerLoop->getStartLoc(),
+                                      InnerLoop->getHeader())
+             << "Loops including conditional statements cannot be interchanged.";
+    });
+    return true;
+  }
+
   // TODO: Currently we handle only loops with 1 induction variable.
   if (Inductions.size() != 1) {
     LLVM_DEBUG(dbgs() << "Loops with more than 1 induction variables are not "


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96706.323718.patch
Type: text/x-patch
Size: 3071 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210215/7d30fec5/attachment.bin>