[PATCH] D144861: [SCEV][IndVars][WIP] Check outer loop invariant when cononicalize comparision

[luxufan via Phabricator via llvm-commits]

StephenFan created this revision.
StephenFan added reviewers: nikic, mkazantsev, wsmoses.
Herald added subscribers: javed.absar, hiraditya.
Herald added a project: All.
StephenFan requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Before this patch, In SimplifyICmpOperands, the comparision would be cononicalized and the NUW
flag would be added if the loop is finite. The reason is as the comments
says, "since the loop is finite, the bound cannot include the corresponding
boundary (otherwise it would loop forever)." But the NUW flag that be added
also assumes the SCEV that this flag attaches is also NUW in outer loops.
It makes some transformations like IndVars eliminating comparisions
that shouldn' t been eliminated.

Fixes: https://github.com/llvm/llvm-project/issues/60944

I am new to ScalarEvolution. So I am not sure if it is a right fix.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144861

Files:
  llvm/include/llvm/Analysis/ScalarEvolution.h
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/test/Transforms/IndVarSimplify/pr60944.ll


Index: llvm/test/Transforms/IndVarSimplify/pr60944.ll
===================================================================
--- llvm/test/Transforms/IndVarSimplify/pr60944.ll
+++ llvm/test/Transforms/IndVarSimplify/pr60944.ll
@@ -32,7 +32,8 @@
 ; CHECK-NEXT:    [[TMP5:%.*]] = load i16, ptr @a, align 4
 ; CHECK-NEXT:    [[CONV5_I:%.*]] = sext i16 [[TMP5]] to i32
 ; CHECK-NEXT:    [[DIV_I]] = sdiv i32 [[CONV5_I]], [[DIV11_I]]
-; CHECK-NEXT:    br i1 false, label [[E_EXIT:%.*]], label [[WHILE_BODY_I]]
+; CHECK-NEXT:    [[CMP6_NOT_I:%.*]] = icmp eq i32 [[DIV11_I]], -1
+; CHECK-NEXT:    br i1 [[CMP6_NOT_I]], label [[E_EXIT:%.*]], label [[WHILE_BODY_I]]
 ; CHECK:       while.body4.i.preheader:
 ; CHECK-NEXT:    br label [[WHILE_BODY4_I]]
 ; CHECK:       while.end.i.loopexit:
Index: llvm/lib/Analysis/ScalarEvolution.cpp
===================================================================
--- llvm/lib/Analysis/ScalarEvolution.cpp
+++ llvm/lib/Analysis/ScalarEvolution.cpp
@@ -9186,9 +9186,9 @@
   bool ControllingFiniteLoop =
       ControlsExit && loopHasNoAbnormalExits(L) && loopIsFiniteByAssumption(L);
   // Simplify the operands before analyzing them.
-  (void)SimplifyICmpOperands(Pred, LHS, RHS, /*Depth=*/0,
-                             (EnableFiniteLoopControl ? ControllingFiniteLoop
-                                                     : false));
+  (void)SimplifyICmpOperands(
+      Pred, LHS, RHS, /*Depth=*/0,
+      (EnableFiniteLoopControl ? ControllingFiniteLoop : false), L);
 
   // If we have a comparison of a chrec against a constant, try to use value
   // ranges to answer this query.
@@ -10655,7 +10655,8 @@
 bool ScalarEvolution::SimplifyICmpOperands(ICmpInst::Predicate &Pred,
                                            const SCEV *&LHS, const SCEV *&RHS,
                                            unsigned Depth,
-                                           bool ControllingFiniteLoop) {
+                                           bool ControllingFiniteLoop,
+                                           const Loop *L) {
   bool Changed = false;
   // Simplifies ICMP to trivial true or false by turning it into '0 == 0' or
   // '0 != 0'.
@@ -10783,6 +10784,12 @@
       return TrivialCase(false);
   }
 
+  bool IsRHSOuterLoopInvariant = false;
+  if (L) {
+    const Loop *Parent = L->getOutermostLoop();
+    IsRHSOuterLoopInvariant = Parent != L ? isLoopInvariant(RHS, Parent) : true;
+  }
+
   // If possible, canonicalize GE/LE comparisons to GT/LT comparisons, by
   // adding or subtracting 1 from one of the operands. This can be done for
   // one of two reasons:
@@ -10818,7 +10825,8 @@
     }
     break;
   case ICmpInst::ICMP_ULE:
-    if (ControllingFiniteLoop || !getUnsignedRangeMax(RHS).isMaxValue()) {
+    if ((ControllingFiniteLoop && IsRHSOuterLoopInvariant) ||
+        !getUnsignedRangeMax(RHS).isMaxValue()) {
       RHS = getAddExpr(getConstant(RHS->getType(), 1, true), RHS,
                        SCEV::FlagNUW);
       Pred = ICmpInst::ICMP_ULT;
Index: llvm/include/llvm/Analysis/ScalarEvolution.h
===================================================================
--- llvm/include/llvm/Analysis/ScalarEvolution.h
+++ llvm/include/llvm/Analysis/ScalarEvolution.h
@@ -1199,7 +1199,8 @@
   /// controls the exit of a loop known to have a finite number of iterations.
   bool SimplifyICmpOperands(ICmpInst::Predicate &Pred, const SCEV *&LHS,
                             const SCEV *&RHS, unsigned Depth = 0,
-                            bool ControllingFiniteLoop = false);
+                            bool ControllingFiniteLoop = false,
+                            const Loop *L = nullptr);
 
   /// Return the "disposition" of the given SCEV with respect to the given
   /// loop.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144861.500733.patch
Type: text/x-patch
Size: 3752 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230227/cf07f23d/attachment.bin>