From 89440df64a5bf9981111e5980647e958f132b6a9 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Mon, 22 Feb 2021 14:34:06 -0800 Subject: [PATCH] [ValueTracking] Improve ComputeNumSignBits for SRem. The result will have the same sign as the dividend unless the result is 0. The magnitude of the result will always be less than or equal to the dividend. So the result will have at least as many sign bits as the dividend. Previously we would do this if the divisor was a positive constant, but that isn't required. Reviewed By: RKSimon Differential Revision: https://reviews.llvm.org/D97170 --- llvm/lib/Analysis/ValueTracking.cpp | 43 +++++++++++------------ llvm/test/Transforms/InstCombine/with_overflow.ll | 6 +--- 2 files changed, 21 insertions(+), 28 deletions(-) diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp index 0491941..520c7f9 100644 --- a/llvm/lib/Analysis/ValueTracking.cpp +++ b/llvm/lib/Analysis/ValueTracking.cpp @@ -2784,6 +2784,8 @@ static unsigned ComputeNumSignBitsImpl(const Value *V, } case Instruction::SRem: { + Tmp = ComputeNumSignBits(U->getOperand(0), Depth + 1, Q); + const APInt *Denominator; // srem X, C -> we know that the result is within [-C+1,C) when C is a // positive constant. This let us put a lower bound on the number of sign @@ -2791,30 +2793,25 @@ static unsigned ComputeNumSignBitsImpl(const Value *V, if (match(U->getOperand(1), m_APInt(Denominator))) { // Ignore non-positive denominator. - if (!Denominator->isStrictlyPositive()) - break; - - // Calculate the incoming numerator bits. SRem by a positive constant - // can't lower the number of sign bits. - unsigned NumrBits = ComputeNumSignBits(U->getOperand(0), Depth + 1, Q); - - // Calculate the leading sign bit constraints by examining the - // denominator. Given that the denominator is positive, there are two - // cases: - // - // 1. the numerator is positive. The result range is [0,C) and [0,C) u< - // (1 << ceilLogBase2(C)). - // - // 2. the numerator is negative. Then the result range is (-C,0] and - // integers in (-C,0] are either 0 or >u (-1 << ceilLogBase2(C)). - // - // Thus a lower bound on the number of sign bits is `TyBits - - // ceilLogBase2(C)`. - - unsigned ResBits = TyBits - Denominator->ceilLogBase2(); - return std::max(NumrBits, ResBits); + if (Denominator->isStrictlyPositive()) { + // Calculate the leading sign bit constraints by examining the + // denominator. Given that the denominator is positive, there are two + // cases: + // + // 1. The numerator is positive. The result range is [0,C) and + // [0,C) u< (1 << ceilLogBase2(C)). + // + // 2. The numerator is negative. Then the result range is (-C,0] and + // integers in (-C,0] are either 0 or >u (-1 << ceilLogBase2(C)). + // + // Thus a lower bound on the number of sign bits is `TyBits - + // ceilLogBase2(C)`. + + unsigned ResBits = TyBits - Denominator->ceilLogBase2(); + Tmp = std::max(Tmp, ResBits); + } } - break; + return Tmp; } case Instruction::AShr: { diff --git a/llvm/test/Transforms/InstCombine/with_overflow.ll b/llvm/test/Transforms/InstCombine/with_overflow.ll index 12db6b7..bff6ce8 100644 --- a/llvm/test/Transforms/InstCombine/with_overflow.ll +++ b/llvm/test/Transforms/InstCombine/with_overflow.ll @@ -332,11 +332,7 @@ define i1 @overflow_mod_overflow_mul(i32 %v1, i32 %v2) nounwind { define i1 @overflow_mod_mul2(i16 %v1, i32 %v2) nounwind { ; CHECK-LABEL: @overflow_mod_mul2( -; CHECK-NEXT: [[A:%.*]] = sext i16 [[V1:%.*]] to i32 -; CHECK-NEXT: [[REM:%.*]] = srem i32 [[A]], [[V2:%.*]] -; CHECK-NEXT: [[T:%.*]] = call { i32, i1 } @llvm.smul.with.overflow.i32(i32 [[REM]], i32 [[REM]]) -; CHECK-NEXT: [[OBIT:%.*]] = extractvalue { i32, i1 } [[T]], 1 -; CHECK-NEXT: ret i1 [[OBIT]] +; CHECK-NEXT: ret i1 false ; %a = sext i16 %v1 to i32 %rem = srem i32 %a, %v2 -- 2.7.4