[SCEV] Improve handling of divisibility information from loop guards. #163021
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
llvm:analysis
Member
|
@llvm/pr-subscribers-llvm-analysis @llvm/pr-subscribers-llvm-transforms Author: Florian Hahn (fhahn) ChangesAt the moment, the effectivness of guards that contain divisibility information (A % B == 0 ) depends on the order of the conditions. This patch makes using divisibility information independent of the order, by collecting and applying the divisibility information separately. We first collect all conditions in a vector, then collect the divisibility information from all guards. When processing other guards, we apply divisibility info collected earlier. After all guards have been processed, we add the divisibility info, rewriting the existing rewrite. This ensures we apply the divisibility info to the largest rewrite expression. This helps to improve results in a few cases, one in dtcxzyw/llvm-opt-benchmark#2921 and another one in a different large C/C++ based IR corpus. Full diff: https://github.com/llvm/llvm-project/pull/163021.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Analysis/ScalarEvolution.h b/llvm/include/llvm/Analysis/ScalarEvolution.h
index 8876e4ed6ae4f..76800d43828c2 100644
--- a/llvm/include/llvm/Analysis/ScalarEvolution.h
+++ b/llvm/include/llvm/Analysis/ScalarEvolution.h
@@ -763,6 +763,10 @@ class ScalarEvolution {
getUMinFromMismatchedTypes(SmallVectorImpl<const SCEV *> &Ops,
bool Sequential = false);
+ /// Try to match the pattern generated by getURemExpr(A, B). If successful,
+ /// Assign A and B to LHS and RHS, respectively.
+ LLVM_ABI bool matchURem(const SCEV *Expr, const SCEV *&LHS, const SCEV *&RHS);
+
/// Transitively follow the chain of pointer-type operands until reaching a
/// SCEV that does not have a single pointer operand. This returns a
/// SCEVUnknown pointer for well-formed pointer-type expressions, but corner
@@ -2316,10 +2320,6 @@ class ScalarEvolution {
/// an add rec on said loop.
void getUsedLoops(const SCEV *S, SmallPtrSetImpl<const Loop *> &LoopsUsed);
- /// Try to match the pattern generated by getURemExpr(A, B). If successful,
- /// Assign A and B to LHS and RHS, respectively.
- LLVM_ABI bool matchURem(const SCEV *Expr, const SCEV *&LHS, const SCEV *&RHS);
-
/// Look for a SCEV expression with type `SCEVType` and operands `Ops` in
/// `UniqueSCEVs`. Return if found, else nullptr.
SCEV *findExistingSCEVInCache(SCEVTypes SCEVType, ArrayRef<const SCEV *> Ops);
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 30bcff7c14923..656ab8d0bdb1a 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -15557,6 +15557,123 @@ void ScalarEvolution::LoopGuards::collectFromPHI(
}
}
+// Checks whether Expr is a non-negative constant, and Divisor is a positive
+// constant, and returns their APInt in ExprVal and in DivisorVal.
+static bool getNonNegExprAndPosDivisor(const SCEV *Expr, const SCEV *Divisor,
+ APInt &ExprVal, APInt &DivisorVal) {
+ auto *ConstExpr = dyn_cast<SCEVConstant>(Expr);
+ auto *ConstDivisor = dyn_cast<SCEVConstant>(Divisor);
+ if (!ConstExpr || !ConstDivisor)
+ return false;
+ ExprVal = ConstExpr->getAPInt();
+ DivisorVal = ConstDivisor->getAPInt();
+ return ExprVal.isNonNegative() && !DivisorVal.isNonPositive();
+}
+
+// Return a new SCEV that modifies \p Expr to the closest number divisible by
+// \p Divisor and less than or equal to Expr.
+// For now, only handle constant Expr and Divisor.
+static const SCEV *getPreviousSCEVDivisibleByDivisor(const SCEV *Expr,
+ const SCEV *Divisor,
+ ScalarEvolution &SE) {
+ APInt ExprVal;
+ APInt DivisorVal;
+ if (!getNonNegExprAndPosDivisor(Expr, Divisor, ExprVal, DivisorVal))
+ return Expr;
+ APInt Rem = ExprVal.urem(DivisorVal);
+ // return the SCEV: Expr - Expr % Divisor
+ return SE.getConstant(ExprVal - Rem);
+}
+
+// Return a new SCEV that modifies \p Expr to the closest number divisible by
+// \p Divisor and greater than or equal to Expr.
+// For now, only handle constant Expr and Divisor.
+static const SCEV *getNextSCEVDivisibleByDivisor(const SCEV *Expr,
+ const SCEV *Divisor,
+ ScalarEvolution &SE) {
+ APInt ExprVal;
+ APInt DivisorVal;
+ if (!getNonNegExprAndPosDivisor(Expr, Divisor, ExprVal, DivisorVal))
+ return Expr;
+ APInt Rem = ExprVal.urem(DivisorVal);
+ if (!Rem.isZero())
+ // return the SCEV: Expr + Divisor - Expr % Divisor
+ return SE.getConstant(ExprVal + DivisorVal - Rem);
+ return Expr;
+}
+
+static bool collectDivisibilityInformation(
+ ICmpInst::Predicate Predicate, const SCEV *LHS, const SCEV *RHS,
+ DenseMap<const SCEV *, const SCEV *> &DivInfo,
+ DenseMap<const SCEV *, const SCEV *> &Multiples, ScalarEvolution &SE) {
+ // If we have LHS == 0, check if LHS is computing a property of some unknown
+ // SCEV %v which we can rewrite %v to express explicitly.
+ if (Predicate != CmpInst::ICMP_EQ || !match(RHS, m_scev_Zero()))
+ return false;
+ // If LHS is A % B, i.e. A % B == 0, rewrite A to (A /u B) * B to
+ // explicitly express that.
+ const SCEV *URemLHS = nullptr;
+ const SCEV *URemRHS = nullptr;
+ if (!SE.matchURem(LHS, URemLHS, URemRHS))
+ return false;
+ if (const SCEVUnknown *LHSUnknown = dyn_cast<SCEVUnknown>(URemLHS)) {
+ const auto *Multiple = SE.getMulExpr(SE.getUDivExpr(LHS, URemRHS), URemRHS);
+ DivInfo[LHSUnknown] = Multiple;
+ Multiples[LHSUnknown] = URemRHS;
+ return true;
+ }
+ return false;
+}
+
+// Check if the condition is a divisibility guard (A % B == 0).
+static bool isDivisibilityGuard(const SCEV *LHS, const SCEV *RHS,
+ ScalarEvolution &SE) {
+ const SCEV *X, *Y;
+ return SE.matchURem(LHS, X, Y) && RHS->isZero();
+}
+
+// Apply divisibility by \p Divisor on MinMaxExpr with constant values,
+// recursively. This is done by aligning up/down the constant value to the
+// Divisor.
+static const SCEV *applyDivisibilityOnMinMaxExpr(const SCEV *MinMaxExpr,
+ const SCEV *Divisor,
+ ScalarEvolution &SE) {
+ // Return true if \p Expr is a MinMax SCEV expression with a non-negative
+ // constant operand. If so, return in \p SCTy the SCEV type and in \p RHS
+ // the non-constant operand and in \p LHS the constant operand.
+ auto IsMinMaxSCEVWithNonNegativeConstant =
+ [](const SCEV *Expr, SCEVTypes &SCTy, const SCEV *&LHS,
+ const SCEV *&RHS) {
+ if (auto *MinMax = dyn_cast<SCEVMinMaxExpr>(Expr)) {
+ if (MinMax->getNumOperands() != 2)
+ return false;
+ if (auto *C = dyn_cast<SCEVConstant>(MinMax->getOperand(0))) {
+ if (C->getAPInt().isNegative())
+ return false;
+ SCTy = MinMax->getSCEVType();
+ LHS = MinMax->getOperand(0);
+ RHS = MinMax->getOperand(1);
+ return true;
+ }
+ }
+ return false;
+ };
+
+ const SCEV *MinMaxLHS = nullptr, *MinMaxRHS = nullptr;
+ SCEVTypes SCTy;
+ if (!IsMinMaxSCEVWithNonNegativeConstant(MinMaxExpr, SCTy, MinMaxLHS,
+ MinMaxRHS))
+ return MinMaxExpr;
+ auto IsMin = isa<SCEVSMinExpr>(MinMaxExpr) || isa<SCEVUMinExpr>(MinMaxExpr);
+ assert(SE.isKnownNonNegative(MinMaxLHS) && "Expected non-negative operand!");
+ auto *DivisibleExpr =
+ IsMin ? getPreviousSCEVDivisibleByDivisor(MinMaxLHS, Divisor, SE)
+ : getNextSCEVDivisibleByDivisor(MinMaxLHS, Divisor, SE);
+ SmallVector<const SCEV *> Ops = {
+ applyDivisibilityOnMinMaxExpr(MinMaxRHS, Divisor, SE), DivisibleExpr};
+ return SE.getMinMaxExpr(SCTy, Ops);
+}
+
void ScalarEvolution::LoopGuards::collectFromBlock(
ScalarEvolution &SE, ScalarEvolution::LoopGuards &Guards,
const BasicBlock *Block, const BasicBlock *Pred,
@@ -15567,19 +15684,14 @@ void ScalarEvolution::LoopGuards::collectFromBlock(
SmallVector<const SCEV *> ExprsToRewrite;
auto CollectCondition = [&](ICmpInst::Predicate Predicate, const SCEV *LHS,
const SCEV *RHS,
- DenseMap<const SCEV *, const SCEV *>
- &RewriteMap) {
+ DenseMap<const SCEV *, const SCEV *> &RewriteMap,
+ const DenseMap<const SCEV *, const SCEV *>
+ &DivInfo) {
// WARNING: It is generally unsound to apply any wrap flags to the proposed
// replacement SCEV which isn't directly implied by the structure of that
// SCEV. In particular, using contextual facts to imply flags is *NOT*
// legal. See the scoping rules for flags in the header to understand why.
- // If LHS is a constant, apply information to the other expression.
- if (isa<SCEVConstant>(LHS)) {
- std::swap(LHS, RHS);
- Predicate = CmpInst::getSwappedPredicate(Predicate);
- }
-
// 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).
@@ -15612,115 +15724,6 @@ void ScalarEvolution::LoopGuards::collectFromBlock(
if (MatchRangeCheckIdiom())
return;
- // Return true if \p Expr is a MinMax SCEV expression with a non-negative
- // constant operand. If so, return in \p SCTy the SCEV type and in \p RHS
- // the non-constant operand and in \p LHS the constant operand.
- auto IsMinMaxSCEVWithNonNegativeConstant =
- [&](const SCEV *Expr, SCEVTypes &SCTy, const SCEV *&LHS,
- const SCEV *&RHS) {
- if (auto *MinMax = dyn_cast<SCEVMinMaxExpr>(Expr)) {
- if (MinMax->getNumOperands() != 2)
- return false;
- if (auto *C = dyn_cast<SCEVConstant>(MinMax->getOperand(0))) {
- if (C->getAPInt().isNegative())
- return false;
- SCTy = MinMax->getSCEVType();
- LHS = MinMax->getOperand(0);
- RHS = MinMax->getOperand(1);
- return true;
- }
- }
- return false;
- };
-
- // Checks whether Expr is a non-negative constant, and Divisor is a positive
- // constant, and returns their APInt in ExprVal and in DivisorVal.
- auto GetNonNegExprAndPosDivisor = [&](const SCEV *Expr, const SCEV *Divisor,
- APInt &ExprVal, APInt &DivisorVal) {
- auto *ConstExpr = dyn_cast<SCEVConstant>(Expr);
- auto *ConstDivisor = dyn_cast<SCEVConstant>(Divisor);
- if (!ConstExpr || !ConstDivisor)
- return false;
- ExprVal = ConstExpr->getAPInt();
- DivisorVal = ConstDivisor->getAPInt();
- return ExprVal.isNonNegative() && !DivisorVal.isNonPositive();
- };
-
- // Return a new SCEV that modifies \p Expr to the closest number divides by
- // \p Divisor and greater or equal than Expr.
- // For now, only handle constant Expr and Divisor.
- auto GetNextSCEVDividesByDivisor = [&](const SCEV *Expr,
- const SCEV *Divisor) {
- APInt ExprVal;
- APInt DivisorVal;
- if (!GetNonNegExprAndPosDivisor(Expr, Divisor, ExprVal, DivisorVal))
- return Expr;
- APInt Rem = ExprVal.urem(DivisorVal);
- if (!Rem.isZero())
- // return the SCEV: Expr + Divisor - Expr % Divisor
- return SE.getConstant(ExprVal + DivisorVal - Rem);
- return Expr;
- };
-
- // Return a new SCEV that modifies \p Expr to the closest number divides by
- // \p Divisor and less or equal than Expr.
- // For now, only handle constant Expr and Divisor.
- auto GetPreviousSCEVDividesByDivisor = [&](const SCEV *Expr,
- const SCEV *Divisor) {
- APInt ExprVal;
- APInt DivisorVal;
- if (!GetNonNegExprAndPosDivisor(Expr, Divisor, ExprVal, DivisorVal))
- return Expr;
- APInt Rem = ExprVal.urem(DivisorVal);
- // return the SCEV: Expr - Expr % Divisor
- return SE.getConstant(ExprVal - Rem);
- };
-
- // Apply divisibilty by \p Divisor on MinMaxExpr with constant values,
- // recursively. This is done by aligning up/down the constant value to the
- // Divisor.
- std::function<const SCEV *(const SCEV *, const SCEV *)>
- ApplyDivisibiltyOnMinMaxExpr = [&](const SCEV *MinMaxExpr,
- const SCEV *Divisor) {
- const SCEV *MinMaxLHS = nullptr, *MinMaxRHS = nullptr;
- SCEVTypes SCTy;
- if (!IsMinMaxSCEVWithNonNegativeConstant(MinMaxExpr, SCTy, MinMaxLHS,
- MinMaxRHS))
- return MinMaxExpr;
- auto IsMin =
- isa<SCEVSMinExpr>(MinMaxExpr) || isa<SCEVUMinExpr>(MinMaxExpr);
- assert(SE.isKnownNonNegative(MinMaxLHS) &&
- "Expected non-negative operand!");
- auto *DivisibleExpr =
- IsMin ? GetPreviousSCEVDividesByDivisor(MinMaxLHS, Divisor)
- : GetNextSCEVDividesByDivisor(MinMaxLHS, Divisor);
- SmallVector<const SCEV *> Ops = {
- ApplyDivisibiltyOnMinMaxExpr(MinMaxRHS, Divisor), DivisibleExpr};
- return SE.getMinMaxExpr(SCTy, Ops);
- };
-
- // If we have LHS == 0, check if LHS is computing a property of some unknown
- // SCEV %v which we can rewrite %v to express explicitly.
- if (Predicate == CmpInst::ICMP_EQ && match(RHS, m_scev_Zero())) {
- // If LHS is A % B, i.e. A % B == 0, rewrite A to (A /u B) * B to
- // explicitly express that.
- const SCEV *URemLHS = nullptr;
- const SCEV *URemRHS = nullptr;
- if (SE.matchURem(LHS, URemLHS, URemRHS)) {
- if (const SCEVUnknown *LHSUnknown = dyn_cast<SCEVUnknown>(URemLHS)) {
- auto I = RewriteMap.find(LHSUnknown);
- const SCEV *RewrittenLHS =
- I != RewriteMap.end() ? I->second : LHSUnknown;
- RewrittenLHS = ApplyDivisibiltyOnMinMaxExpr(RewrittenLHS, URemRHS);
- const auto *Multiple =
- SE.getMulExpr(SE.getUDivExpr(RewrittenLHS, URemRHS), URemRHS);
- RewriteMap[LHSUnknown] = Multiple;
- ExprsToRewrite.push_back(LHSUnknown);
- return;
- }
- }
- }
-
// Do not apply information for constants or if RHS contains an AddRec.
if (isa<SCEVConstant>(LHS) || SE.containsAddRecurrence(RHS))
return;
@@ -15751,7 +15754,11 @@ void ScalarEvolution::LoopGuards::collectFromBlock(
const SCEV *RewrittenLHS = GetMaybeRewritten(LHS);
const SCEV *DividesBy = nullptr;
- const APInt &Multiple = SE.getConstantMultiple(RewrittenLHS);
+ // Apply divisibility information when computing the constant multiple.
+ LoopGuards DivGuards(SE);
+ DivGuards.RewriteMap = DivInfo;
+ const APInt &Multiple =
+ SE.getConstantMultiple(DivGuards.rewrite(RewrittenLHS));
if (!Multiple.isOne())
DividesBy = SE.getConstant(Multiple);
@@ -15775,21 +15782,23 @@ void ScalarEvolution::LoopGuards::collectFromBlock(
[[fallthrough]];
case CmpInst::ICMP_SLT: {
RHS = SE.getMinusSCEV(RHS, One);
- RHS = DividesBy ? GetPreviousSCEVDividesByDivisor(RHS, DividesBy) : RHS;
+ RHS = DividesBy ? getPreviousSCEVDivisibleByDivisor(RHS, DividesBy, SE)
+ : RHS;
break;
}
case CmpInst::ICMP_UGT:
case CmpInst::ICMP_SGT:
RHS = SE.getAddExpr(RHS, One);
- RHS = DividesBy ? GetNextSCEVDividesByDivisor(RHS, DividesBy) : RHS;
+ RHS = DividesBy ? getNextSCEVDivisibleByDivisor(RHS, DividesBy, SE) : RHS;
break;
case CmpInst::ICMP_ULE:
case CmpInst::ICMP_SLE:
- RHS = DividesBy ? GetPreviousSCEVDividesByDivisor(RHS, DividesBy) : RHS;
+ RHS = DividesBy ? getPreviousSCEVDivisibleByDivisor(RHS, DividesBy, SE)
+ : RHS;
break;
case CmpInst::ICMP_UGE:
case CmpInst::ICMP_SGE:
- RHS = DividesBy ? GetNextSCEVDividesByDivisor(RHS, DividesBy) : RHS;
+ RHS = DividesBy ? getNextSCEVDivisibleByDivisor(RHS, DividesBy, SE) : RHS;
break;
default:
break;
@@ -15843,7 +15852,8 @@ void ScalarEvolution::LoopGuards::collectFromBlock(
case CmpInst::ICMP_NE:
if (match(RHS, m_scev_Zero())) {
const SCEV *OneAlignedUp =
- DividesBy ? GetNextSCEVDividesByDivisor(One, DividesBy) : One;
+ DividesBy ? getNextSCEVDivisibleByDivisor(One, DividesBy, SE)
+ : One;
To = SE.getUMaxExpr(FromRewritten, OneAlignedUp);
}
break;
@@ -15916,8 +15926,11 @@ void ScalarEvolution::LoopGuards::collectFromBlock(
// Now apply the information from the collected conditions to
// Guards.RewriteMap. Conditions are processed in reverse order, so the
- // earliest conditions is processed first. This ensures the SCEVs with the
+ // earliest conditions is processed first, except guards with divisibility
+ // information, which are moved to the back. This ensures the SCEVs with the
// shortest dependency chains are constructed first.
+ SmallVector<std::tuple<CmpInst::Predicate, const SCEV *, const SCEV *>>
+ GuardsToProcess;
for (auto [Term, EnterIfTrue] : reverse(Terms)) {
SmallVector<Value *, 8> Worklist;
SmallPtrSet<Value *, 8> Visited;
@@ -15932,7 +15945,12 @@ void ScalarEvolution::LoopGuards::collectFromBlock(
EnterIfTrue ? Cmp->getPredicate() : Cmp->getInversePredicate();
const auto *LHS = SE.getSCEV(Cmp->getOperand(0));
const auto *RHS = SE.getSCEV(Cmp->getOperand(1));
- CollectCondition(Predicate, LHS, RHS, Guards.RewriteMap);
+ // If LHS is a constant, apply information to the other expression.
+ if (isa<SCEVConstant>(LHS)) {
+ std::swap(LHS, RHS);
+ Predicate = CmpInst::getSwappedPredicate(Predicate);
+ }
+ GuardsToProcess.emplace_back(Predicate, LHS, RHS);
continue;
}
@@ -15945,6 +15963,28 @@ void ScalarEvolution::LoopGuards::collectFromBlock(
}
}
+ // Process divisibility guards in reverse order to populate DivInfo early.
+ DenseMap<const SCEV *, const SCEV *> Multiples;
+ DenseMap<const SCEV *, const SCEV *> DivInfo;
+ for (const auto &[Predicate, LHS, RHS] : GuardsToProcess) {
+ if (!isDivisibilityGuard(LHS, RHS, SE))
+ continue;
+ collectDivisibilityInformation(Predicate, LHS, RHS, DivInfo, Multiples, SE);
+ }
+
+ for (const auto &[Predicate, LHS, RHS] : GuardsToProcess)
+ CollectCondition(Predicate, LHS, RHS, Guards.RewriteMap, DivInfo);
+
+ // Apply divisibility information last. This ensures it is applied to the
+ // outermost expression after other rewrites for the given value.
+ for (const auto &[K, V] : Multiples) {
+ Guards.RewriteMap[K] = SE.getMulExpr(
+ SE.getUDivExpr(applyDivisibilityOnMinMaxExpr(Guards.rewrite(K), V, SE),
+ V),
+ V);
+ ExprsToRewrite.push_back(K);
+ }
+
// Let the rewriter preserve NUW/NSW flags if the unsigned/signed ranges of
// the replacement expressions are contained in the ranges of the replaced
// expressions.
diff --git a/llvm/test/Transforms/IndVarSimplify/loop-guard-order.ll b/llvm/test/Transforms/IndVarSimplify/loop-guard-order.ll
index 14ee00d77197c..2763860e79875 100644
--- a/llvm/test/Transforms/IndVarSimplify/loop-guard-order.ll
+++ b/llvm/test/Transforms/IndVarSimplify/loop-guard-order.ll
@@ -114,7 +114,7 @@ define i32 @urem_order1(i32 %n) {
; CHECK: [[LOOP]]:
; CHECK-NEXT: [[IV:%.*]] = phi i32 [ [[IV_NEXT:%.*]], %[[LOOP]] ], [ 0, %[[LOOP_PREHEADER]] ]
; CHECK-NEXT: call void @foo()
-; CHECK-NEXT: [[IV_NEXT]] = add i32 [[IV]], 3
+; CHECK-NEXT: [[IV_NEXT]] = add nuw i32 [[IV]], 3
; CHECK-NEXT: [[EC:%.*]] = icmp eq i32 [[IV_NEXT]], [[N]]
; CHECK-NEXT: br i1 [[EC]], label %[[EXIT_LOOPEXIT:.*]], label %[[LOOP]]
; CHECK: [[EXIT_LOOPEXIT]]:
@@ -205,13 +205,12 @@ define i64 @test_loop_with_div_order_1(i64 %n) {
; CHECK-NEXT: [[PARITY_CHECK:%.*]] = icmp eq i64 [[IS_ODD]], 0
; CHECK-NEXT: br i1 [[PARITY_CHECK]], label %[[LOOP_PREHEADER:.*]], label %[[EXIT]]
; CHECK: [[LOOP_PREHEADER]]:
-; CHECK-NEXT: [[UMAX:%.*]] = call i64 @llvm.umax.i64(i64 [[UPPER_BOUND]], i64 1)
; CHECK-NEXT: br label %[[LOOP:.*]]
; CHECK: [[LOOP]]:
; CHECK-NEXT: [[IV:%.*]] = phi i64 [ [[IV_NEXT:%.*]], %[[LOOP]] ], [ 0, %[[LOOP_PREHEADER]] ]
; CHECK-NEXT: [[DUMMY:%.*]] = load volatile i64, ptr null, align 8
; CHECK-NEXT: [[IV_NEXT]] = add nuw nsw i64 [[IV]], 1
-; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i64 [[IV_NEXT]], [[UMAX]]
+; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i64 [[IV_NEXT]], [[UPPER_BOUND]]
; CHECK-NEXT: br i1 [[EXITCOND]], label %[[LOOP]], label %[[EXIT_LOOPEXIT:.*]]
; CHECK: [[EXIT_LOOPEXIT]]:
; CHECK-NEXT: br label %[[EXIT]]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| const auto *LHS = SE.getSCEV(Cmp->getOperand(0)); | ||
| const auto *RHS = SE.getSCEV(Cmp->getOperand(1)); | ||
| CollectCondition(Predicate, LHS, RHS, Guards.RewriteMap); | ||
| // If LHS is a constant, apply information to the other expression. |
Collaborator
There was a problem hiding this comment.
And if LHS is not a constant, we make an arbitrary choice? Should we be using CompareSCEVComplexity or something?
Contributor
Author
There was a problem hiding this comment.
Yep, that currently just preserves the current behavior. I added a TODO
| const APInt &Multiple = SE.getConstantMultiple(RewrittenLHS); | ||
| // Apply divisibility information when computing the constant multiple. | ||
| LoopGuards DivGuards(SE); | ||
| DivGuards.RewriteMap = DivInfo; |
Collaborator
There was a problem hiding this comment.
Is this copying the map?
Contributor
Author
There was a problem hiding this comment.
Yep, I adjusted that by constructing DivGuards once up-front, thanks!
fhahn
force-pushed
the
scev-guards-div-first
branch
3 times, most recently
from
7e0c2dd to
6893dad
Compare
This was referenced Oct 20, 2025
fhahn
added a commit
that referenced
this pull request
Oct 20, 2025
Add test with urem guard with non-constant divisor and AddRec guards. Extra test coverage for #163021
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Oct 20, 2025
Add test with urem guard with non-constant divisor and AddRec guards. Extra test coverage for llvm/llvm-project#163021
fhahn
added a commit
that referenced
this pull request
Oct 20, 2025
Move getPreviousSCEVDivisibleByDivisor from a lambda to a static function and clarify the name (DividesBy -> DivisibleBy). Split off refactoring from #163021.
fhahn
force-pushed
the
scev-guards-div-first
branch
from
6893dad to
568b2af
Compare
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Oct 20, 2025
… (NFC). Move getPreviousSCEVDivisibleByDivisor from a lambda to a static function and clarify the name (DividesBy -> DivisibleBy). Split off refactoring from llvm/llvm-project#163021.
fhahn
force-pushed
the
scev-guards-div-first
branch
2 times, most recently
from
d73ea19 to
49ec971
Compare
Contributor
Author
|
ping |
Lukacma
pushed a commit
to Lukacma/llvm-project
that referenced
this pull request
Oct 29, 2025
Add test with urem guard with non-constant divisor and AddRec guards. Extra test coverage for llvm#163021
Lukacma
pushed a commit
to Lukacma/llvm-project
that referenced
this pull request
Oct 29, 2025
Move getPreviousSCEVDivisibleByDivisor from a lambda to a static function and clarify the name (DividesBy -> DivisibleBy). Split off refactoring from llvm#163021.
aokblast
pushed a commit
to aokblast/llvm-project
that referenced
this pull request
Oct 30, 2025
Add test with urem guard with non-constant divisor and AddRec guards. Extra test coverage for llvm#163021
aokblast
pushed a commit
to aokblast/llvm-project
that referenced
this pull request
Oct 30, 2025
Move getPreviousSCEVDivisibleByDivisor from a lambda to a static function and clarify the name (DividesBy -> DivisibleBy). Split off refactoring from llvm#163021.
fhahn
added a commit
that referenced
this pull request
Oct 31, 2025
Fix formatting for switch, to avoid unrelated changes/formatting errors in #163021.
fhahn
force-pushed
the
scev-guards-div-first
branch
from
49ec971 to
60fe65a
Compare
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Oct 31, 2025
Fix formatting for switch, to avoid unrelated changes/formatting errors in llvm/llvm-project#163021.
At the moment, the effectivness of guards that contain divisibility information (A % B == 0 ) depends on the order of the conditions. This patch makes using divisibility information independent of the order, by collecting and applying the divisibility information separately. We first collect all conditions in a vector, then collect the divisibility information from all guards. When processing other guards, we apply divisibility info collected earlier. After all guards have been processed, we add the divisibility info, rewriting the existing rewrite. This ensures we apply the divisibility info to the largest rewrite expression. This helps to improve results in a few cases, one in dtcxzyw/llvm-opt-benchmark#2921 and another one in a different large C/C++ based IR corpus.
fhahn
force-pushed
the
scev-guards-div-first
branch
from
60fe65a to
11aea0e
Compare
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/26871 Here is the relevant piece of the build log for the reference |
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Nov 2, 2025
…oop guards. (#163021) At the moment, the effectivness of guards that contain divisibility information (A % B == 0 ) depends on the order of the conditions. This patch makes using divisibility information independent of the order, by collecting and applying the divisibility information separately. We first collect all conditions in a vector, then collect the divisibility information from all guards. When processing other guards, we apply divisibility info collected earlier. After all guards have been processed, we add the divisibility info, rewriting the existing rewrite. This ensures we apply the divisibility info to the largest rewrite expression. This helps to improve results in a few cases, one in dtcxzyw/llvm-opt-benchmark#2921 and another one in a different large C/C++ based IR corpus. PR: llvm/llvm-project#163021
DEBADRIBASAK
pushed a commit
to DEBADRIBASAK/llvm-project
that referenced
this pull request
Nov 3, 2025
Fix formatting for switch, to avoid unrelated changes/formatting errors in llvm#163021.
DEBADRIBASAK
pushed a commit
to DEBADRIBASAK/llvm-project
that referenced
this pull request
Nov 3, 2025
…llvm#163021) At the moment, the effectivness of guards that contain divisibility information (A % B == 0 ) depends on the order of the conditions. This patch makes using divisibility information independent of the order, by collecting and applying the divisibility information separately. We first collect all conditions in a vector, then collect the divisibility information from all guards. When processing other guards, we apply divisibility info collected earlier. After all guards have been processed, we add the divisibility info, rewriting the existing rewrite. This ensures we apply the divisibility info to the largest rewrite expression. This helps to improve results in a few cases, one in dtcxzyw/llvm-opt-benchmark#2921 and another one in a different large C/C++ based IR corpus. PR: llvm#163021
fhahn
added a commit
to fhahn/llvm-project
that referenced
this pull request
Nov 4, 2025
Add test with urem guard with non-constant divisor and AddRec guards. Extra test coverage for llvm#163021 (cherry picked from commit 0731f18)
fhahn
added a commit
to fhahn/llvm-project
that referenced
this pull request
Nov 4, 2025
Move getPreviousSCEVDivisibleByDivisor from a lambda to a static function and clarify the name (DividesBy -> DivisibleBy). Split off refactoring from llvm#163021. (cherry picked from commit 385ea0d)
fhahn
added a commit
to fhahn/llvm-project
that referenced
this pull request
Nov 4, 2025
Fix formatting for switch, to avoid unrelated changes/formatting errors in llvm#163021. (cherry picked from commit 817b7c5)
fhahn
added a commit
to fhahn/llvm-project
that referenced
this pull request
Nov 4, 2025
…llvm#163021) At the moment, the effectivness of guards that contain divisibility information (A % B == 0 ) depends on the order of the conditions. This patch makes using divisibility information independent of the order, by collecting and applying the divisibility information separately. We first collect all conditions in a vector, then collect the divisibility information from all guards. When processing other guards, we apply divisibility info collected earlier. After all guards have been processed, we add the divisibility info, rewriting the existing rewrite. This ensures we apply the divisibility info to the largest rewrite expression. This helps to improve results in a few cases, one in dtcxzyw/llvm-opt-benchmark#2921 and another one in a different large C/C++ based IR corpus. PR: llvm#163021 (cherry picked from commit d3fe1df)
ckoparkar
pushed a commit
to ckoparkar/llvm-project
that referenced
this pull request
Nov 6, 2025
Fix formatting for switch, to avoid unrelated changes/formatting errors in llvm#163021.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
At the moment, the effectivness of guards that contain divisibility information (A % B == 0 ) depends on the order of the conditions.
This patch makes using divisibility information independent of the order, by collecting and applying the divisibility information separately.
We first collect all conditions in a vector, then collect the divisibility information from all guards.
When processing other guards, we apply divisibility info collected earlier.
After all guards have been processed, we add the divisibility info, rewriting the existing rewrite. This ensures we apply the divisibility info to the largest rewrite expression.
This helps to improve results in a few cases, one in dtcxzyw/llvm-opt-benchmark#2921 and another one in a different large C/C++ based IR corpus.