caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[Analysis] allow caller to choose signed/unsigned when computing constant range

Browse Source 
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
This commit is contained in:
Sanjay Patel
2021-12-28 09:25:16 -05:00
parent aaeae842ef
commit 0edf99950e
8 changed files with 55 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
llvm/include/llvm/Analysis/ValueTracking.h
View File

@@ -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,

4
llvm/lib/Analysis/BasicAliasAnalysis.cpp
View File

@@ -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(

3
llvm/lib/Analysis/InstructionSimplify.cpp
View File

@@ -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);

24
llvm/lib/Analysis/ValueTracking.cpp
View File

@@ -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));
}

3
llvm/lib/Transforms/Vectorize/VectorCombine.cpp
View File

@@ -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();
}

12
llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
View File

@@ -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
}

16
llvm/test/Transforms/InstSimplify/icmp-constant.ll
View File

@@ -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(

20
llvm/unittests/Analysis/ValueTrackingTest.cpp
View File

@@ -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());
}