[SCEV] Add initial pattern matching for SCEV constants. (NFC)

[fhahn]

Add initial pattern matching for SCEV constants. Follow-up patches will add additional matchers for various SCEV expressions.

This patch only converts a few instances to use the new matchers to make sure everything builds as expected for now.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: Florian Hahn (fhahn)

Changes

Add initial pattern matching for SCEV constants. Follow-up patches will add additional matchers for various SCEV expressions.

This patch only converts a few instances to use the new matchers to make sure everything builds as expected for now.

---

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

2 Files Affected:

• (added) llvm/include/llvm/Analysis/ScalarEvolutionPatternMatch.h (+59)
• (modified) llvm/lib/Analysis/ScalarEvolution.cpp (+7-6)

diff --git a/llvm/include/llvm/Analysis/ScalarEvolutionPatternMatch.h b/llvm/include/llvm/Analysis/ScalarEvolutionPatternMatch.h
new file mode 100644
index 00000000000000..636b9f8e1544f7
--- /dev/null
+++ b/llvm/include/llvm/Analysis/ScalarEvolutionPatternMatch.h
@@ -0,0 +1,59 @@
+//===- ScalarEvolutionPatternMatch.h - Match on SCEVs -----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file provides a simple and efficient mechanism for performing general
+// tree-based pattern matches on SCEVs, based on LLVM's IR pattern matchers.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_ANALYSIS_SCALAREVOLUTIONPATTERNMATCH_H
+#define LLVM_ANALYSIS_SCALAREVOLUTIONPATTERNMATCH_H
+
+#include "llvm/Analysis/ScalarEvolutionExpressions.h"
+
+namespace llvm {
+namespace SCEVPatternMatch {
+
+template <typename Val, typename Pattern>
+bool match(const SCEV *S, const Pattern &P) {
+  return P.match(S);
+}
+
+/// Match a specified integer value. \p BitWidth optionally specifies the
+/// bitwidth the matched constant must have. If it is 0, the matched constant
+/// can have any bitwidth.
+template <unsigned BitWidth = 0> struct specific_intval {
+  APInt Val;
+
+  specific_intval(APInt V) : Val(std::move(V)) {}
+
+  bool match(const SCEV *S) const {
+    const auto *C = dyn_cast<SCEVConstant>(S);
+    if (!C)
+      return false;
+
+    if (BitWidth != 0 && C->getAPInt().getBitWidth() != BitWidth)
+      return false;
+    return APInt::isSameValue(C->getAPInt(), Val);
+  }
+};
+
+inline specific_intval<0> m_scev_Zero() {
+  return specific_intval<0>(APInt(64, 0));
+}
+inline specific_intval<0> m_scev_One() {
+  return specific_intval<0>(APInt(64, 1));
+}
+inline specific_intval<0> m_scev_MinusOne() {
+  return specific_intval<0>(APInt(64, -1));
+}
+
+} // namespace SCEVPatternMatch
+} // namespace llvm
+
+#endif
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index cad10486cbf3fa..741431ac8aa158 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -79,6 +79,7 @@
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/MemoryBuiltins.h"
 #include "llvm/Analysis/ScalarEvolutionExpressions.h"
+#include "llvm/Analysis/ScalarEvolutionPatternMatch.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/Config/llvm-config.h"
@@ -133,6 +134,7 @@
 
 using namespace llvm;
 using namespace PatternMatch;
+using namespace SCEVPatternMatch;
 
 #define DEBUG_TYPE "scalar-evolution"
 
@@ -3423,9 +3425,8 @@ const SCEV *ScalarEvolution::getUDivExpr(const SCEV *LHS,
     return S;
 
   // 0 udiv Y == 0
-  if (const SCEVConstant *LHSC = dyn_cast<SCEVConstant>(LHS))
-    if (LHSC->getValue()->isZero())
-      return LHS;
+  if (match(LHS, m_scev_Zero()))
+    return LHS;
 
   if (const SCEVConstant *RHSC = dyn_cast<SCEVConstant>(RHS)) {
     if (RHSC->getValue()->isOne())
@@ -10593,7 +10594,6 @@ ScalarEvolution::ExitLimit ScalarEvolution::howFarToZero(const SCEV *V,
   // Get the initial value for the loop.
   const SCEV *Start = getSCEVAtScope(AddRec->getStart(), L->getParentLoop());
   const SCEV *Step = getSCEVAtScope(AddRec->getOperand(1), L->getParentLoop());
-  const SCEVConstant *StepC = dyn_cast<SCEVConstant>(Step);
 
   if (!isLoopInvariant(Step, L))
     return getCouldNotCompute();
@@ -10615,8 +10615,8 @@ ScalarEvolution::ExitLimit ScalarEvolution::howFarToZero(const SCEV *V,
   // Handle unitary steps, which cannot wraparound.
   // 1*N = -Start; -1*N = Start (mod 2^BW), so:
   //   N = Distance (as unsigned)
-  if (StepC &&
-      (StepC->getValue()->isOne() || StepC->getValue()->isMinusOne())) {
+
+  if (match(Step, m_CombineOr(m_scev_One(), m_scev_MinusOne()))) {
     APInt MaxBECount = getUnsignedRangeMax(applyLoopGuards(Distance, Guards));
     MaxBECount = APIntOps::umin(MaxBECount, getUnsignedRangeMax(Distance));
 
@@ -10668,6 +10668,7 @@ ScalarEvolution::ExitLimit ScalarEvolution::howFarToZero(const SCEV *V,
   }
 
   // Solve the general equation.
+  const SCEVConstant *StepC = dyn_cast<SCEVConstant>(Step);
   if (!StepC || StepC->getValue()->isZero())
     return getCouldNotCompute();
   const SCEV *E = SolveLinEquationWithOverflow(

[nikic]

I like the general idea.

[nikic]

This is going to miscompile > 64-bit due to missing sign extension. Please implement these via predicates like in PatternMatch, rather than using isSameValue and bypassing the checks that exist to prevent this mistake.

[fhahn]

Yeah, updated to build on top of PatternMatch::specific_intval64, thanks

[nikic]

This doesn't really address the problem. You'll still not get the correct sign extension behavior here.

What I expected to see is something along these lines:

template <typename Predicate>
struct cst_pred_ty : public Predicate {
  bool match(const SCEV *S) {
    auto *C = dyn_cast<SCEVConstant>(S);
    return C && isValue(C->getAPInt());
  }
}

inline cst_pred_ty<PatternMatch::is_all_ones> m_scev_AllOnes() {
  return cst_pred_ty<PatternMatch::is_all_ones>();
}

[nikic]

(Can also copy is_all_ones etc if we don't want the PatternMatch dependency here.)

[fhahn]

Ah I see, I misunderstood originally. Should be updated, thanks

[nikic]

Per recent changes to guidelines, you can drop the file name and the emacs marker.

[fhahn]

Replaced with just ```/===----------------------------------------------------------------------===//`

[nikic]

Suggested change

	/// Match an integer 0 or a vector with all elements equal to 0.
	/// For vectors, this includes constants with undefined elements.
	/// Match an integer 0.

SCEV does not operate on vectors at all.

Also, I don't think we really want the Zero/ZeroInt distinction in SCEV? If we really wanted to match "0 or null" we'd have to check for a SCEVUnknown with a null pointer constant, but I don't think this is something that we'd want to do when working on SCEV.

[fhahn]

Yep, simplified thanks, and added an assert that we don't encounter vector types.

[nikic]

I still think this one should use cst_pred_ty<is_zero>. Your current implementation is equivalent to doing that, in a somewhat roundabout way.

[fhahn]

Updated, thanks

[nikic]

LGTM