[llvm] [SCEV][NFC] Refactor LoopGuards range check matching (PR #210362)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-analysis

Author: Aleksandr Popov (aleks-tmb)

<details>
<summary>Changes</summary>

Parameterize MatchRangeCheckIdiom over (Predicate, LHS, RHS) so it can be reused for other predicates discovered during guard collection.

Hoist AddRewrite / GetMaybeRewritten above it and use them in place of the inline try_emplace, so range-check rewrites chain onto existing ones the same way other rewrites do.

---
Full diff: https://github.com/llvm/llvm-project/pull/210362.diff


1 Files Affected:

- (modified) llvm/lib/Analysis/ScalarEvolution.cpp (+27-28) 


``````````diff
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 134be6ac097e0..e6450d647da8a 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -16014,11 +16014,29 @@ void ScalarEvolution::LoopGuards::collectFromBlock(
     // SCEV.  In particular, using contextual facts to imply flags is *NOT*
     // legal.  See the scoping rules for flags in the header to understand why.
 
+    // Puts rewrite rule \p From -> \p To into the rewrite map. Also if \p From
+    // and \p FromRewritten are the same (i.e. there has been no rewrite
+    // registered for \p From), then puts this value in the list of rewritten
+    // expressions.
+    auto AddRewrite = [&](const SCEV *From, const SCEV *FromRewritten,
+                          const SCEV *To) {
+      if (From == FromRewritten)
+        ExprsToRewrite.push_back(From);
+      RewriteMap[From] = To;
+    };
+
+    // Checks whether \p S has already been rewritten. In that case returns the
+    // existing rewrite because we want to chain further rewrites onto the
+    // already rewritten value. Otherwise returns \p S.
+    auto GetMaybeRewritten = [&](const SCEV *S) {
+      return RewriteMap.lookup_or(S, S);
+    };
+
     // Check for a condition of the form (-C1 + X < C2).  InstCombine will
     // create this form when combining two checks of the form (X u< C2 + C1) and
     // (X >=u C1).
-    auto MatchRangeCheckIdiom = [&SE, Predicate, LHS, RHS, &RewriteMap,
-                                 &ExprsToRewrite]() {
+    auto MatchRangeCheckIdiom = [&](ICmpInst::Predicate Predicate,
+                                    const SCEV *LHS, const SCEV *RHS) {
       const SCEVConstant *C1;
       const SCEVUnknown *LHSUnknown;
       auto *C2 = dyn_cast<SCEVConstant>(RHS);
@@ -16034,16 +16052,15 @@ void ScalarEvolution::LoopGuards::collectFromBlock(
       // Bail out, unless we have a non-wrapping, monotonic range.
       if (ExactRegion.isWrappedSet() || ExactRegion.isFullSet())
         return false;
-      auto [I, Inserted] = RewriteMap.try_emplace(LHSUnknown);
-      const SCEV *RewrittenLHS = Inserted ? LHSUnknown : I->second;
-      I->second = SE.getUMaxExpr(
-          SE.getConstant(ExactRegion.getUnsignedMin()),
-          SE.getUMinExpr(RewrittenLHS,
-                         SE.getConstant(ExactRegion.getUnsignedMax())));
-      ExprsToRewrite.push_back(LHSUnknown);
+      const SCEV *RewrittenLHS = GetMaybeRewritten(LHSUnknown);
+      const SCEV *RegionMin = SE.getConstant(ExactRegion.getUnsignedMin());
+      const SCEV *RegionMax = SE.getConstant(ExactRegion.getUnsignedMax());
+      const SCEV *ClampedLHS =
+          SE.getUMaxExpr(RegionMin, SE.getUMinExpr(RewrittenLHS, RegionMax));
+      AddRewrite(LHSUnknown, RewrittenLHS, ClampedLHS);
       return true;
     };
-    if (MatchRangeCheckIdiom())
+    if (MatchRangeCheckIdiom(Predicate, LHS, RHS))
       return;
 
     // Do not apply information for constants or if RHS contains an AddRec.
@@ -16056,24 +16073,6 @@ void ScalarEvolution::LoopGuards::collectFromBlock(
       Predicate = CmpInst::getSwappedPredicate(Predicate);
     }
 
-    // Puts rewrite rule \p From -> \p To into the rewrite map. Also if \p From
-    // and \p FromRewritten are the same (i.e. there has been no rewrite
-    // registered for \p From), then puts this value in the list of rewritten
-    // expressions.
-    auto AddRewrite = [&](const SCEV *From, const SCEV *FromRewritten,
-                          const SCEV *To) {
-      if (From == FromRewritten)
-        ExprsToRewrite.push_back(From);
-      RewriteMap[From] = To;
-    };
-
-    // Checks whether \p S has already been rewritten. In that case returns the
-    // existing rewrite because we want to chain further rewrites onto the
-    // already rewritten value. Otherwise returns \p S.
-    auto GetMaybeRewritten = [&](const SCEV *S) {
-      return RewriteMap.lookup_or(S, S);
-    };
-
     const SCEV *RewrittenLHS = GetMaybeRewritten(LHS);
     // Apply divisibility information when computing the constant multiple.
     const APInt &DividesBy =

``````````

</details>


https://github.com/llvm/llvm-project/pull/210362