diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h --- a/llvm/include/llvm/Analysis/ValueTracking.h +++ b/llvm/include/llvm/Analysis/ValueTracking.h @@ -555,7 +555,8 @@ /// 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 --- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp +++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp @@ -1248,8 +1248,8 @@ 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 --- a/llvm/lib/Analysis/InstructionSimplify.cpp +++ b/llvm/lib/Analysis/InstructionSimplify.cpp @@ -2890,7 +2890,8 @@ 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 --- a/llvm/lib/Analysis/ValueTracking.cpp +++ b/llvm/lib/Analysis/ValueTracking.cpp @@ -6756,7 +6756,8 @@ } 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 @@ 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 @@ } } -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 @@ APInt Lower = APInt(BitWidth, 0); APInt Upper = APInt(BitWidth, 0); if (auto *BO = dyn_cast(V)) - setLimitsForBinOp(*BO, Lower, Upper, IIQ); + setLimitsForBinOp(*BO, Lower, Upper, IIQ, ForSigned); else if (auto *II = dyn_cast(V)) setLimitsForIntrinsic(*II, Lower, Upper); else if (auto *SI = dyn_cast(V)) @@ -7136,8 +7144,10 @@ // 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 --- a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp +++ b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp @@ -881,7 +881,8 @@ 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 --- a/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll +++ b/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll @@ -2129,3 +2129,15 @@ %r = call <3 x i8> @llvm.umax.v3i8(<3 x i8> %a, <3 x i8> ) 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 --- a/llvm/test/Transforms/InstSimplify/icmp-constant.ll +++ b/llvm/test/Transforms/InstSimplify/icmp-constant.ll @@ -632,18 +632,18 @@ 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 @@ 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 --- a/llvm/unittests/Analysis/ValueTrackingTest.cpp +++ b/llvm/unittests/Analysis/ValueTrackingTest.cpp @@ -2000,11 +2000,11 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ // 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 @@ 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()); }