[llvm] 0edf999 - [Analysis] allow caller to choose signed/unsigned when computing constant range
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 28 06:52:04 PST 2021
Author: Sanjay Patel
Date: 2021-12-28T09:45:37-05:00
New Revision: 0edf99950e6234159c99710838f21d3629d756af
URL: https://github.com/llvm/llvm-project/commit/0edf99950e6234159c99710838f21d3629d756af
DIFF: https://github.com/llvm/llvm-project/commit/0edf99950e6234159c99710838f21d3629d756af.diff
LOG: [Analysis] allow caller to choose signed/unsigned when computing constant range
We should not lose analysis precision if an 'add' has both no-wrap
flags (nsw and nuw) compared to just one or the other.
This patch is modeled on a similar construct that was added with
D59386.
I don't think it is possible to expose a problem with an unsigned
compare because of the way this was coded (nuw is handled first).
InstCombine has an assert that fires with the example from:
https://github.com/llvm/llvm-project/issues/52884
...because it was expecting InstSimplify to handle this kind of
pattern with an smax.
Fixes #52884
Differential Revision: https://reviews.llvm.org/D116322
Added:
Modified:
llvm/include/llvm/Analysis/ValueTracking.h
llvm/lib/Analysis/BasicAliasAnalysis.cpp
llvm/lib/Analysis/InstructionSimplify.cpp
llvm/lib/Analysis/ValueTracking.cpp
llvm/lib/Transforms/Vectorize/VectorCombine.cpp
llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
llvm/test/Transforms/InstSimplify/icmp-constant.ll
llvm/unittests/Analysis/ValueTrackingTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h
index b4f38a3e976fd..f0f78c0eaed4e 100644
--- a/llvm/include/llvm/Analysis/ValueTracking.h
+++ b/llvm/include/llvm/Analysis/ValueTracking.h
@@ -555,7 +555,8 @@ constexpr unsigned MaxAnalysisRecursionDepth = 6;
/// Determine the possible constant range of an integer or vector of integer
/// value. This is intended as a cheap, non-recursive check.
- ConstantRange computeConstantRange(const Value *V, bool UseInstrInfo = true,
+ ConstantRange computeConstantRange(const Value *V, bool ForSigned,
+ bool UseInstrInfo = true,
AssumptionCache *AC = nullptr,
const Instruction *CtxI = nullptr,
const DominatorTree *DT = nullptr,
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index 5f1bf2001d470..6f0da2cf18fab 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1248,8 +1248,8 @@ AliasResult BasicAAResult::aliasGEP(
else
GCD = APIntOps::GreatestCommonDivisor(GCD, ScaleForGCD.abs());
- ConstantRange CR =
- computeConstantRange(Index.Val.V, true, &AC, Index.CxtI);
+ ConstantRange CR = computeConstantRange(Index.Val.V, /* ForSigned */ false,
+ true, &AC, Index.CxtI);
KnownBits Known =
computeKnownBits(Index.Val.V, DL, 0, &AC, Index.CxtI, DT);
CR = CR.intersectWith(
diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 4831b22b1d469..1c26ab3619089 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -2890,7 +2890,8 @@ static Value *simplifyICmpWithConstant(CmpInst::Predicate Pred, Value *LHS,
if (RHS_CR.isFullSet())
return ConstantInt::getTrue(ITy);
- ConstantRange LHS_CR = computeConstantRange(LHS, IIQ.UseInstrInfo);
+ ConstantRange LHS_CR =
+ computeConstantRange(LHS, CmpInst::isSigned(Pred), IIQ.UseInstrInfo);
if (!LHS_CR.isFullSet()) {
if (RHS_CR.contains(LHS_CR))
return ConstantInt::getTrue(ITy);
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 7a1caed0420a1..7876e209acc69 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -6756,7 +6756,8 @@ Optional<bool> llvm::isImpliedByDomCondition(CmpInst::Predicate Pred,
}
static void setLimitsForBinOp(const BinaryOperator &BO, APInt &Lower,
- APInt &Upper, const InstrInfoQuery &IIQ) {
+ APInt &Upper, const InstrInfoQuery &IIQ,
+ bool PreferSignedRange) {
unsigned Width = Lower.getBitWidth();
const APInt *C;
switch (BO.getOpcode()) {
@@ -6764,7 +6765,14 @@ static void setLimitsForBinOp(const BinaryOperator &BO, APInt &Lower,
if (match(BO.getOperand(1), m_APInt(C)) && !C->isZero()) {
bool HasNSW = IIQ.hasNoSignedWrap(&BO);
bool HasNUW = IIQ.hasNoUnsignedWrap(&BO);
- // FIXME: If we have both nuw and nsw, we should reduce the range further.
+
+ // If the caller expects a signed compare, then try to use a signed range.
+ // Otherwise if both no-wraps are set, use the unsigned range because it
+ // is never larger than the signed range. Example:
+ // "add nuw nsw i8 X, -2" is unsigned [254,255] vs. signed [-128, 125].
+ if (PreferSignedRange && HasNSW && HasNUW)
+ HasNUW = false;
+
if (HasNUW) {
// 'add nuw x, C' produces [C, UINT_MAX].
Lower = *C;
@@ -7085,8 +7093,8 @@ static void setLimitForFPToI(const Instruction *I, APInt &Lower, APInt &Upper) {
}
}
-ConstantRange llvm::computeConstantRange(const Value *V, bool UseInstrInfo,
- AssumptionCache *AC,
+ConstantRange llvm::computeConstantRange(const Value *V, bool ForSigned,
+ bool UseInstrInfo, AssumptionCache *AC,
const Instruction *CtxI,
const DominatorTree *DT,
unsigned Depth) {
@@ -7104,7 +7112,7 @@ ConstantRange llvm::computeConstantRange(const Value *V, bool UseInstrInfo,
APInt Lower = APInt(BitWidth, 0);
APInt Upper = APInt(BitWidth, 0);
if (auto *BO = dyn_cast<BinaryOperator>(V))
- setLimitsForBinOp(*BO, Lower, Upper, IIQ);
+ setLimitsForBinOp(*BO, Lower, Upper, IIQ, ForSigned);
else if (auto *II = dyn_cast<IntrinsicInst>(V))
setLimitsForIntrinsic(*II, Lower, Upper);
else if (auto *SI = dyn_cast<SelectInst>(V))
@@ -7136,8 +7144,10 @@ ConstantRange llvm::computeConstantRange(const Value *V, bool UseInstrInfo,
// Currently we just use information from comparisons.
if (!Cmp || Cmp->getOperand(0) != V)
continue;
- ConstantRange RHS = computeConstantRange(Cmp->getOperand(1), UseInstrInfo,
- AC, I, DT, Depth + 1);
+ // TODO: Set "ForSigned" parameter via Cmp->isSigned()?
+ ConstantRange RHS =
+ computeConstantRange(Cmp->getOperand(1), UseInstrInfo,
+ /* ForSigned */ false, AC, I, DT, Depth + 1);
CR = CR.intersectWith(
ConstantRange::makeAllowedICmpRegion(Cmp->getPredicate(), RHS));
}
diff --git a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
index c0aedab2fed0d..620d388199e0f 100644
--- a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
+++ b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
@@ -881,7 +881,8 @@ static ScalarizationResult canScalarizeAccess(FixedVectorType *VecTy,
ConstantRange IdxRange(IntWidth, true);
if (isGuaranteedNotToBePoison(Idx, &AC)) {
- if (ValidIndices.contains(computeConstantRange(Idx, true, &AC, CtxI, &DT)))
+ if (ValidIndices.contains(computeConstantRange(Idx, /* ForSigned */ false,
+ true, &AC, CtxI, &DT)))
return ScalarizationResult::safe();
return ScalarizationResult::unsafe();
}
diff --git a/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll b/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
index 7b76c3ad74226..194a693e55804 100644
--- a/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
+++ b/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
@@ -2129,3 +2129,15 @@ define <3 x i8> @umax_vector_splat_undef(<3 x i8> %x) {
%r = call <3 x i8> @llvm.umax.v3i8(<3 x i8> %a, <3 x i8> <i8 13, i8 130, i8 130>)
ret <3 x i8> %r
}
+
+; Issue #52884 - this would assert because of a failure to simplify.
+
+define i8 @smax_offset_simplify(i8 %x) {
+; CHECK-LABEL: @smax_offset_simplify(
+; CHECK-NEXT: [[TMP1:%.*]] = add nuw nsw i8 [[X:%.*]], 50
+; CHECK-NEXT: ret i8 [[TMP1]]
+;
+ %1 = add nuw nsw i8 50, %x
+ %m = call i8 @llvm.smax.i8(i8 %1, i8 -124)
+ ret i8 %m
+}
diff --git a/llvm/test/Transforms/InstSimplify/icmp-constant.ll b/llvm/test/Transforms/InstSimplify/icmp-constant.ll
index 0784510a7ae40..46ac6f6b9dacc 100644
--- a/llvm/test/Transforms/InstSimplify/icmp-constant.ll
+++ b/llvm/test/Transforms/InstSimplify/icmp-constant.ll
@@ -632,18 +632,18 @@ define i1 @add_nsw_sgt(i8 %x) {
ret i1 %cmp
}
+; nuw should not inhibit the fold.
+
define i1 @add_nsw_nuw_sgt(i8 %x) {
; CHECK-LABEL: @add_nsw_nuw_sgt(
-; CHECK-NEXT: [[ADD:%.*]] = add nuw nsw i8 [[X:%.*]], 5
-; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i8 [[ADD]], -124
-; CHECK-NEXT: ret i1 [[CMP]]
+; CHECK-NEXT: ret i1 true
;
%add = add nsw nuw i8 %x, 5
%cmp = icmp sgt i8 %add, -124
ret i1 %cmp
}
-; minimum x is -128, so add could be -124.
+; negative test - minimum x is -128, so add could be -124.
define i1 @add_nsw_sgt_limit(i8 %x) {
; CHECK-LABEL: @add_nsw_sgt_limit(
@@ -665,18 +665,18 @@ define i1 @add_nsw_slt(i8 %x) {
ret i1 %cmp
}
+; nuw should not inhibit the fold.
+
define i1 @add_nsw_nuw_slt(i8 %x) {
; CHECK-LABEL: @add_nsw_nuw_slt(
-; CHECK-NEXT: [[ADD:%.*]] = add nuw nsw i8 [[X:%.*]], 5
-; CHECK-NEXT: [[CMP:%.*]] = icmp slt i8 [[ADD]], -123
-; CHECK-NEXT: ret i1 [[CMP]]
+; CHECK-NEXT: ret i1 false
;
%add = add nsw nuw i8 %x, 5
%cmp = icmp slt i8 %add, -123
ret i1 %cmp
}
-; minimum x is -128, so add could be -123.
+; negative test - minimum x is -128, so add could be -123.
define i1 @add_nsw_slt_limit(i8 %x) {
; CHECK-LABEL: @add_nsw_slt_limit(
diff --git a/llvm/unittests/Analysis/ValueTrackingTest.cpp b/llvm/unittests/Analysis/ValueTrackingTest.cpp
index 0104d321f2c94..f4b4d345a0541 100644
--- a/llvm/unittests/Analysis/ValueTrackingTest.cpp
+++ b/llvm/unittests/Analysis/ValueTrackingTest.cpp
@@ -2000,11 +2000,11 @@ TEST_F(ValueTrackingTest, ComputeConstantRange) {
AssumptionCache AC(*F);
Value *Stride = &*F->arg_begin();
- ConstantRange CR1 = computeConstantRange(Stride, true, &AC, nullptr);
+ ConstantRange CR1 = computeConstantRange(Stride, false, true, &AC, nullptr);
EXPECT_TRUE(CR1.isFullSet());
Instruction *I = &findInstructionByName(F, "stride.plus.one");
- ConstantRange CR2 = computeConstantRange(Stride, true, &AC, I);
+ ConstantRange CR2 = computeConstantRange(Stride, false, true, &AC, I);
EXPECT_EQ(5, CR2.getLower());
EXPECT_EQ(10, CR2.getUpper());
}
@@ -2034,7 +2034,7 @@ TEST_F(ValueTrackingTest, ComputeConstantRange) {
AssumptionCache AC(*F);
Value *Stride = &*F->arg_begin();
Instruction *I = &findInstructionByName(F, "stride.plus.one");
- ConstantRange CR = computeConstantRange(Stride, true, &AC, I);
+ ConstantRange CR = computeConstantRange(Stride, false, true, &AC, I);
EXPECT_EQ(99, *CR.getSingleElement());
}
@@ -2072,12 +2072,12 @@ TEST_F(ValueTrackingTest, ComputeConstantRange) {
AssumptionCache AC(*F);
Value *Stride = &*F->arg_begin();
Instruction *GT2 = &findInstructionByName(F, "gt.2");
- ConstantRange CR = computeConstantRange(Stride, true, &AC, GT2);
+ ConstantRange CR = computeConstantRange(Stride, false, true, &AC, GT2);
EXPECT_EQ(5, CR.getLower());
EXPECT_EQ(0, CR.getUpper());
Instruction *I = &findInstructionByName(F, "stride.plus.one");
- ConstantRange CR2 = computeConstantRange(Stride, true, &AC, I);
+ ConstantRange CR2 = computeConstantRange(Stride, false, true, &AC, I);
EXPECT_EQ(50, CR2.getLower());
EXPECT_EQ(100, CR2.getUpper());
}
@@ -2105,7 +2105,7 @@ TEST_F(ValueTrackingTest, ComputeConstantRange) {
Value *Stride = &*F->arg_begin();
Instruction *I = &findInstructionByName(F, "stride.plus.one");
- ConstantRange CR = computeConstantRange(Stride, true, &AC, I);
+ ConstantRange CR = computeConstantRange(Stride, false, true, &AC, I);
EXPECT_TRUE(CR.isEmptySet());
}
@@ -2133,8 +2133,8 @@ TEST_F(ValueTrackingTest, ComputeConstantRange) {
Value *X2 = &*std::next(F->arg_begin());
Instruction *I = &findInstructionByName(F, "stride.plus.one");
- ConstantRange CR1 = computeConstantRange(X1, true, &AC, I);
- ConstantRange CR2 = computeConstantRange(X2, true, &AC, I);
+ ConstantRange CR1 = computeConstantRange(X1, false, true, &AC, I);
+ ConstantRange CR2 = computeConstantRange(X2, false, true, &AC, I);
EXPECT_EQ(5, CR1.getLower());
EXPECT_EQ(0, CR1.getUpper());
@@ -2144,7 +2144,7 @@ TEST_F(ValueTrackingTest, ComputeConstantRange) {
// Check the depth cutoff results in a conservative result (full set) by
// passing Depth == MaxDepth == 6.
- ConstantRange CR3 = computeConstantRange(X2, true, &AC, I, nullptr, 6);
+ ConstantRange CR3 = computeConstantRange(X2, false, true, &AC, I, nullptr, 6);
EXPECT_TRUE(CR3.isFullSet());
}
{
@@ -2165,7 +2165,7 @@ TEST_F(ValueTrackingTest, ComputeConstantRange) {
Value *X2 = &*std::next(F->arg_begin());
Instruction *I = &findInstructionByName(F, "stride.plus.one");
- ConstantRange CR1 = computeConstantRange(X2, true, &AC, I);
+ ConstantRange CR1 = computeConstantRange(X2, false, true, &AC, I);
// If we don't know the value of x.2, we don't know the value of x.1.
EXPECT_TRUE(CR1.isFullSet());
}
More information about the llvm-commits
mailing list