From a60832b0187642d01fd726dc766cd62587f6add0 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 13 Aug 2008 23:12:35 +0000 Subject: [PATCH] Fix a bogus srem rule - a negative value srem'd by a power-of-2 can have a non-negative result; for example, -16%16 is 0. Also, clarify the related comments. This fixes PR2670. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@54767 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/ValueTracking.cpp | 8 +++----- lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 8 +++----- lib/Transforms/Scalar/InstructionCombining.cpp | 3 --- test/Transforms/InstCombine/srem1.ll | 18 ++++++++++++++++++ 4 files changed, 24 insertions(+), 13 deletions(-) create mode 100644 test/Transforms/InstCombine/srem1.ll diff --git a/lib/Analysis/ValueTracking.cpp b/lib/Analysis/ValueTracking.cpp index e35f0d0fced..3a04f5eb869 100644 --- a/lib/Analysis/ValueTracking.cpp +++ b/lib/Analysis/ValueTracking.cpp @@ -369,15 +369,13 @@ void llvm::ComputeMaskedBits(Value *V, const APInt &Mask, ComputeMaskedBits(I->getOperand(0), Mask2, KnownZero2, KnownOne2, TD, Depth+1); - // The sign of a remainder is equal to the sign of the first - // operand (zero being positive). + // If the sign bit of the first operand is zero, the sign bit of + // the result is zero. If the first operand has no one bits below + // the second operand's single 1 bit, its sign will be zero. if (KnownZero2[BitWidth-1] || ((KnownZero2 & LowBits) == LowBits)) KnownZero2 |= ~LowBits; - else if (KnownOne2[BitWidth-1]) - KnownOne2 |= ~LowBits; KnownZero |= KnownZero2 & Mask; - KnownOne |= KnownOne2 & Mask; assert((KnownZero & KnownOne) == 0&&"Bits known to be one AND zero?"); } diff --git a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp index 144dc6e198f..9eab89ffb03 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp +++ b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp @@ -1656,15 +1656,13 @@ void SelectionDAG::ComputeMaskedBits(SDValue Op, const APInt &Mask, APInt Mask2 = LowBits | APInt::getSignBit(BitWidth); ComputeMaskedBits(Op.getOperand(0), Mask2,KnownZero2,KnownOne2,Depth+1); - // The sign of a remainder is equal to the sign of the first - // operand (zero being positive). + // If the sign bit of the first operand is zero, the sign bit of + // the result is zero. If the first operand has no one bits below + // the second operand's single 1 bit, its sign will be zero. if (KnownZero2[BitWidth-1] || ((KnownZero2 & LowBits) == LowBits)) KnownZero2 |= ~LowBits; - else if (KnownOne2[BitWidth-1]) - KnownOne2 |= ~LowBits; KnownZero |= KnownZero2 & Mask; - KnownOne |= KnownOne2 & Mask; assert((KnownZero & KnownOne) == 0&&"Bits known to be one AND zero?"); } diff --git a/lib/Transforms/Scalar/InstructionCombining.cpp b/lib/Transforms/Scalar/InstructionCombining.cpp index bdea6e40d21..74b88b9afc6 100644 --- a/lib/Transforms/Scalar/InstructionCombining.cpp +++ b/lib/Transforms/Scalar/InstructionCombining.cpp @@ -1266,11 +1266,8 @@ bool InstCombiner::SimplifyDemandedBits(Value *V, APInt DemandedMask, if (LHSKnownZero[BitWidth-1] || ((LHSKnownZero & LowBits) == LowBits)) LHSKnownZero |= ~LowBits; - else if (LHSKnownOne[BitWidth-1]) - LHSKnownOne |= ~LowBits; KnownZero |= LHSKnownZero & DemandedMask; - KnownOne |= LHSKnownOne & DemandedMask; assert((KnownZero & KnownOne) == 0&&"Bits known to be one AND zero?"); } diff --git a/test/Transforms/InstCombine/srem1.ll b/test/Transforms/InstCombine/srem1.ll new file mode 100644 index 00000000000..ee59d3ed99f --- /dev/null +++ b/test/Transforms/InstCombine/srem1.ll @@ -0,0 +1,18 @@ +; RUN: llvm-as < %s | opt -instcombine +; PR2670 + +@g_127 = external global i32 ; [#uses=1] + +define i32 @func_56(i32 %p_58, i32 %p_59, i32 %p_61, i16 signext %p_62) nounwind { +entry: + %call = call i32 (...)* @rshift_s_s( i32 %p_61, i32 1 ) ; [#uses=1] + %conv = sext i32 %call to i64 ; [#uses=1] + %or = or i64 -1734012817166602727, %conv ; [#uses=1] + %rem = srem i64 %or, 1 ; [#uses=1] + %cmp = icmp eq i64 %rem, 1 ; [#uses=1] + %cmp.ext = zext i1 %cmp to i32 ; [#uses=1] + store i32 %cmp.ext, i32* @g_127 + ret i32 undef +} + +declare i32 @rshift_s_s(...)