[llvm] [ConstraintElimination] Add eq/ne facts to signed constraint system (PR #121423)
Stephen Senran Zhang via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 31 20:58:23 PST 2024
https://github.com/zsrkmyn created https://github.com/llvm/llvm-project/pull/121423
Facts of eq/ne were added to unsigned system only, causing some missing
optimizations. This patch adds eq/ne facts to both signed & unsigned
constraint system.
Fixes #117961.
>From e86f74eb4e925c053f52abce54291c8644d89ab9 Mon Sep 17 00:00:00 2001
From: Senran Zhang <zsrkmyn at gmail.com>
Date: Wed, 1 Jan 2025 11:58:53 +0800
Subject: [PATCH 1/2] [NFC][ConstraintElimination] Optimize code styles
This patch does following things,
- prefer early exits;
- add missing std::move;
- avoid duplicate map lookups;
- prefer emplace_back to avoid unnecessary copies.
---
.../Scalar/ConstraintElimination.cpp | 75 +++++++++----------
1 file changed, 37 insertions(+), 38 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
index ead07ed37f215f..91a3c3f0d392a1 100644
--- a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
@@ -216,7 +216,7 @@ struct StackEntry {
StackEntry(unsigned NumIn, unsigned NumOut, bool IsSigned,
SmallVector<Value *, 2> ValuesToRelease)
: NumIn(NumIn), NumOut(NumOut), IsSigned(IsSigned),
- ValuesToRelease(ValuesToRelease) {}
+ ValuesToRelease(std::move(ValuesToRelease)) {}
};
struct ConstraintTy {
@@ -726,8 +726,8 @@ ConstraintInfo::getConstraint(CmpInst::Predicate Pred, Value *Op0, Value *Op1,
}
for (const auto &KV : VariablesB) {
- if (SubOverflow(R[GetOrAddIndex(KV.Variable)], KV.Coefficient,
- R[GetOrAddIndex(KV.Variable)]))
+ auto &Coeff = R[GetOrAddIndex(KV.Variable)];
+ if (SubOverflow(Coeff, KV.Coefficient, Coeff))
return {};
auto I =
KnownNonNegativeVariables.insert({KV.Variable, KV.IsKnownNonNegative});
@@ -759,9 +759,9 @@ ConstraintInfo::getConstraint(CmpInst::Predicate Pred, Value *Op0, Value *Op1,
if (!KV.second ||
(!Value2Index.contains(KV.first) && !NewIndexMap.contains(KV.first)))
continue;
- SmallVector<int64_t, 8> C(Value2Index.size() + NewVariables.size() + 1, 0);
+ auto &C = Res.ExtraInfo.emplace_back(
+ Value2Index.size() + NewVariables.size() + 1, 0);
C[GetOrAddIndex(KV.first)] = -1;
- Res.ExtraInfo.push_back(C);
}
return Res;
}
@@ -1591,53 +1591,52 @@ void ConstraintInfo::addFact(CmpInst::Predicate Pred, Value *A, Value *B,
LLVM_DEBUG(dbgs() << "Adding '"; dumpUnpackedICmp(dbgs(), Pred, A, B);
dbgs() << "'\n");
- bool Added = false;
auto &CSToUse = getCS(R.IsSigned);
if (R.Coefficients.empty())
return;
- Added |= CSToUse.addVariableRowFill(R.Coefficients);
+ bool Added = CSToUse.addVariableRowFill(R.Coefficients);
+ if (!Added)
+ return;
// If R has been added to the system, add the new variables and queue it for
// removal once it goes out-of-scope.
- if (Added) {
- SmallVector<Value *, 2> ValuesToRelease;
- auto &Value2Index = getValue2Index(R.IsSigned);
- for (Value *V : NewVariables) {
- Value2Index.insert({V, Value2Index.size() + 1});
- ValuesToRelease.push_back(V);
- }
-
- LLVM_DEBUG({
- dbgs() << " constraint: ";
- dumpConstraint(R.Coefficients, getValue2Index(R.IsSigned));
- dbgs() << "\n";
- });
+ SmallVector<Value *, 2> ValuesToRelease;
+ auto &Value2Index = getValue2Index(R.IsSigned);
+ for (Value *V : NewVariables) {
+ Value2Index.insert({V, Value2Index.size() + 1});
+ ValuesToRelease.push_back(V);
+ }
- DFSInStack.emplace_back(NumIn, NumOut, R.IsSigned,
- std::move(ValuesToRelease));
-
- if (!R.IsSigned) {
- for (Value *V : NewVariables) {
- ConstraintTy VarPos(SmallVector<int64_t, 8>(Value2Index.size() + 1, 0),
- false, false, false);
- VarPos.Coefficients[Value2Index[V]] = -1;
- CSToUse.addVariableRow(VarPos.Coefficients);
- DFSInStack.emplace_back(NumIn, NumOut, R.IsSigned,
- SmallVector<Value *, 2>());
- }
- }
+ LLVM_DEBUG({
+ dbgs() << " constraint: ";
+ dumpConstraint(R.Coefficients, getValue2Index(R.IsSigned));
+ dbgs() << "\n";
+ });
- if (R.isEq()) {
- // Also add the inverted constraint for equality constraints.
- for (auto &Coeff : R.Coefficients)
- Coeff *= -1;
- CSToUse.addVariableRowFill(R.Coefficients);
+ DFSInStack.emplace_back(NumIn, NumOut, R.IsSigned,
+ std::move(ValuesToRelease));
+ if (!R.IsSigned) {
+ for (Value *V : NewVariables) {
+ ConstraintTy VarPos(SmallVector<int64_t, 8>(Value2Index.size() + 1, 0),
+ false, false, false);
+ VarPos.Coefficients[Value2Index[V]] = -1;
+ CSToUse.addVariableRow(VarPos.Coefficients);
DFSInStack.emplace_back(NumIn, NumOut, R.IsSigned,
SmallVector<Value *, 2>());
}
}
+
+ if (R.isEq()) {
+ // Also add the inverted constraint for equality constraints.
+ for (auto &Coeff : R.Coefficients)
+ Coeff *= -1;
+ CSToUse.addVariableRowFill(R.Coefficients);
+
+ DFSInStack.emplace_back(NumIn, NumOut, R.IsSigned,
+ SmallVector<Value *, 2>());
+ }
}
static bool replaceSubOverflowUses(IntrinsicInst *II, Value *A, Value *B,
>From 777c92653c93de8a116dad0903039049e9f44307 Mon Sep 17 00:00:00 2001
From: Senran Zhang <zsrkmyn at gmail.com>
Date: Wed, 1 Jan 2025 12:26:21 +0800
Subject: [PATCH 2/2] [ConstraintElimination] Add eq/ne facts to signed
constraint system
Facts of eq/ne were added to unsigned system only, causing some missing
optimizations. This patch adds eq/ne facts to both signed & unsigned
constraint system.
Fixes #117961.
---
.../Scalar/ConstraintElimination.cpp | 38 +++++++++++++++----
.../Transforms/ConstraintElimination/ne.ll | 6 +--
.../ConstraintElimination/pr105785.ll | 3 +-
.../ConstraintElimination/pr117961.ll | 22 +++++++++++
4 files changed, 56 insertions(+), 13 deletions(-)
create mode 100644 llvm/test/Transforms/ConstraintElimination/pr117961.ll
diff --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
index 91a3c3f0d392a1..1c086d01a18c4c 100644
--- a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
@@ -313,7 +313,8 @@ class ConstraintInfo {
/// New variables that need to be added to the system are collected in
/// \p NewVariables.
ConstraintTy getConstraint(CmpInst::Predicate Pred, Value *Op0, Value *Op1,
- SmallVectorImpl<Value *> &NewVariables) const;
+ SmallVectorImpl<Value *> &NewVariables,
+ bool ForceSignedSystem = false) const;
/// Turns a comparison of the form \p Op0 \p Pred \p Op1 into a vector of
/// constraints using getConstraint. Returns an empty constraint if the result
@@ -330,6 +331,14 @@ class ConstraintInfo {
void transferToOtherSystem(CmpInst::Predicate Pred, Value *A, Value *B,
unsigned NumIn, unsigned NumOut,
SmallVectorImpl<StackEntry> &DFSInStack);
+
+private:
+ /// Adds facts into constraint system. \p ForceSignedSystem can be set when
+ /// the \p Pred is eq/ne, and signed constraint system is used when it's
+ /// specified.
+ void addFactImpl(CmpInst::Predicate Pred, Value *A, Value *B, unsigned NumIn,
+ unsigned NumOut, SmallVectorImpl<StackEntry> &DFSInStack,
+ bool ForceSignedSystem);
};
/// Represents a (Coefficient * Variable) entry after IR decomposition.
@@ -636,8 +645,13 @@ static Decomposition decompose(Value *V,
ConstraintTy
ConstraintInfo::getConstraint(CmpInst::Predicate Pred, Value *Op0, Value *Op1,
- SmallVectorImpl<Value *> &NewVariables) const {
+ SmallVectorImpl<Value *> &NewVariables,
+ bool ForceSignedSystem) const {
assert(NewVariables.empty() && "NewVariables must be empty when passed in");
+ assert((!ForceSignedSystem || Pred == CmpInst::ICMP_EQ ||
+ Pred == CmpInst::ICMP_NE) &&
+ "signed system can only be forced on eq/ne");
+
bool IsEq = false;
bool IsNe = false;
@@ -652,7 +666,7 @@ ConstraintInfo::getConstraint(CmpInst::Predicate Pred, Value *Op0, Value *Op1,
break;
}
case CmpInst::ICMP_EQ:
- if (match(Op1, m_Zero())) {
+ if (!ForceSignedSystem && match(Op1, m_Zero())) {
Pred = CmpInst::ICMP_ULE;
} else {
IsEq = true;
@@ -660,7 +674,7 @@ ConstraintInfo::getConstraint(CmpInst::Predicate Pred, Value *Op0, Value *Op1,
}
break;
case CmpInst::ICMP_NE:
- if (match(Op1, m_Zero())) {
+ if (!ForceSignedSystem && match(Op1, m_Zero())) {
Pred = CmpInst::getSwappedPredicate(CmpInst::ICMP_UGT);
std::swap(Op0, Op1);
} else {
@@ -677,7 +691,7 @@ ConstraintInfo::getConstraint(CmpInst::Predicate Pred, Value *Op0, Value *Op1,
return {};
SmallVector<ConditionTy, 4> Preconditions;
- bool IsSigned = CmpInst::isSigned(Pred);
+ bool IsSigned = ForceSignedSystem || CmpInst::isSigned(Pred);
auto &Value2Index = getValue2Index(IsSigned);
auto ADec = decompose(Op0->stripPointerCastsSameRepresentation(),
Preconditions, IsSigned, DL);
@@ -737,7 +751,7 @@ ConstraintInfo::getConstraint(CmpInst::Predicate Pred, Value *Op0, Value *Op1,
int64_t OffsetSum;
if (AddOverflow(Offset1, Offset2, OffsetSum))
return {};
- if (Pred == (IsSigned ? CmpInst::ICMP_SLT : CmpInst::ICMP_ULT))
+ if (Pred == CmpInst::ICMP_SLT || Pred == CmpInst::ICMP_ULT)
if (AddOverflow(OffsetSum, int64_t(-1), OffsetSum))
return {};
R[0] = OffsetSum;
@@ -1580,10 +1594,20 @@ static bool checkOrAndOpImpliedByOther(
void ConstraintInfo::addFact(CmpInst::Predicate Pred, Value *A, Value *B,
unsigned NumIn, unsigned NumOut,
SmallVectorImpl<StackEntry> &DFSInStack) {
+ addFactImpl(Pred, A, B, NumIn, NumOut, DFSInStack, false);
+ // If the Pred is eq/ne, also add the fact to signed system.
+ if (Pred == ICmpInst::ICMP_EQ || Pred == ICmpInst::ICMP_NE)
+ addFactImpl(Pred, A, B, NumIn, NumOut, DFSInStack, true);
+}
+
+void ConstraintInfo::addFactImpl(CmpInst::Predicate Pred, Value *A, Value *B,
+ unsigned NumIn, unsigned NumOut,
+ SmallVectorImpl<StackEntry> &DFSInStack,
+ bool ForceSignedSystem) {
// If the constraint has a pre-condition, skip the constraint if it does not
// hold.
SmallVector<Value *> NewVariables;
- auto R = getConstraint(Pred, A, B, NewVariables);
+ auto R = getConstraint(Pred, A, B, NewVariables, ForceSignedSystem);
// TODO: Support non-equality for facts as well.
if (!R.isValid(*this) || R.isNe())
diff --git a/llvm/test/Transforms/ConstraintElimination/ne.ll b/llvm/test/Transforms/ConstraintElimination/ne.ll
index 566e73dc8d626e..4753860db2851b 100644
--- a/llvm/test/Transforms/ConstraintElimination/ne.ll
+++ b/llvm/test/Transforms/ConstraintElimination/ne.ll
@@ -71,8 +71,7 @@ define i1 @test_ne_eq_0(i8 %a, i8 %b) {
; CHECK-NEXT: [[RES_13:%.*]] = xor i1 [[RES_12]], false
; CHECK-NEXT: [[RES_14:%.*]] = xor i1 [[RES_13]], false
; CHECK-NEXT: [[RES_15:%.*]] = xor i1 [[RES_14]], false
-; CHECK-NEXT: [[C_12:%.*]] = icmp sgt i8 [[A]], 0
-; CHECK-NEXT: [[RES_16:%.*]] = xor i1 [[RES_15]], [[C_12]]
+; CHECK-NEXT: [[RES_16:%.*]] = xor i1 [[RES_15]], false
; CHECK-NEXT: ret i1 [[RES_16]]
;
entry:
@@ -209,8 +208,7 @@ define i1 @test_ne_eq_1(i8 %a, i8 %b) {
; CHECK-NEXT: [[RES_13:%.*]] = xor i1 [[RES_12]], true
; CHECK-NEXT: [[RES_14:%.*]] = xor i1 [[RES_13]], true
; CHECK-NEXT: [[RES_15:%.*]] = xor i1 [[RES_14]], false
-; CHECK-NEXT: [[C_12:%.*]] = icmp sgt i8 [[A]], 0
-; CHECK-NEXT: [[RES_16:%.*]] = xor i1 [[RES_15]], [[C_12]]
+; CHECK-NEXT: [[RES_16:%.*]] = xor i1 [[RES_15]], true
; CHECK-NEXT: ret i1 [[RES_16]]
;
entry:
diff --git a/llvm/test/Transforms/ConstraintElimination/pr105785.ll b/llvm/test/Transforms/ConstraintElimination/pr105785.ll
index 6c340a11dd2e2c..83b7461720f09c 100644
--- a/llvm/test/Transforms/ConstraintElimination/pr105785.ll
+++ b/llvm/test/Transforms/ConstraintElimination/pr105785.ll
@@ -15,8 +15,7 @@ define void @pr105785(ptr %p) {
; CHECK-NEXT: [[CMP2:%.*]] = icmp ult i32 [[FOR_IND2]], 3
; CHECK-NEXT: br i1 [[CMP2]], label %[[FOR_BODY3]], label %[[FOR_COND]]
; CHECK: [[FOR_BODY3]]:
-; CHECK-NEXT: [[SCMP:%.*]] = call i32 @llvm.scmp.i32.i32(i32 [[FOR_IND]], i32 1)
-; CHECK-NEXT: store i32 [[SCMP]], ptr [[P]], align 4
+; CHECK-NEXT: store i32 -1, ptr [[P]], align 4
; CHECK-NEXT: [[INC]] = add nuw nsw i32 [[FOR_IND2]], 1
; CHECK-NEXT: br label %[[FOR_COND1]]
; CHECK: [[FOR_END6]]:
diff --git a/llvm/test/Transforms/ConstraintElimination/pr117961.ll b/llvm/test/Transforms/ConstraintElimination/pr117961.ll
new file mode 100644
index 00000000000000..346196a04b0014
--- /dev/null
+++ b/llvm/test/Transforms/ConstraintElimination/pr117961.ll
@@ -0,0 +1,22 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -passes=constraint-elimination -S %s | FileCheck %s
+
+define i1 @f(i32 noundef %v0, i32 noundef %v1, i32 noundef %v2) {
+; CHECK-LABEL: define i1 @f(
+; CHECK-SAME: i32 noundef [[V0:%.*]], i32 noundef [[V1:%.*]], i32 noundef [[V2:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[V2]], [[V0]]
+; CHECK-NEXT: [[CMP1:%.*]] = icmp sge i32 [[V0]], [[V1]]
+; CHECK-NEXT: [[AND0:%.*]] = and i1 [[CMP1]], [[CMP]]
+; CHECK-NEXT: [[CMP4:%.*]] = icmp sgt i32 [[V1]], [[V2]]
+; CHECK-NEXT: [[AND1:%.*]] = and i1 false, [[AND0]]
+; CHECK-NEXT: ret i1 [[AND1]]
+;
+entry:
+ %cmp = icmp eq i32 %v2, %v0
+ %cmp1 = icmp sge i32 %v0, %v1
+ %and0 = and i1 %cmp1, %cmp
+ %cmp4 = icmp sgt i32 %v1, %v2
+ %and1 = and i1 %cmp4, %and0
+ ret i1 %and1
+}
More information about the llvm-commits
mailing list