From 70635adea36679b98bbc87f196548ed42a244a29 Mon Sep 17 00:00:00 2001 From: Bill Wendling Date: Sun, 30 Nov 2008 03:42:12 +0000 Subject: [PATCH] Instcombine was illegally transforming -X/C into X/-C when either X or C overflowed on negation. This commit checks to make sure that neithe C nor X overflows. This requires that the RHS of X (a subtract instruction) be a constant integer. llvm-svn: 60275 --- .../Scalar/InstructionCombining.cpp | 23 ++++++++++++++++--- llvm/test/Transforms/InstCombine/apint-sub.ll | 8 ++++--- llvm/test/Transforms/InstCombine/sdiv-1.ll | 22 ++++++++++++++++++ llvm/test/Transforms/InstCombine/sub.ll | 8 ++++--- 4 files changed, 52 insertions(+), 9 deletions(-) create mode 100644 llvm/test/Transforms/InstCombine/sdiv-1.ll diff --git a/llvm/lib/Transforms/Scalar/InstructionCombining.cpp b/llvm/lib/Transforms/Scalar/InstructionCombining.cpp index 9214b6bbd115..8461aa75e028 100644 --- a/llvm/lib/Transforms/Scalar/InstructionCombining.cpp +++ b/llvm/lib/Transforms/Scalar/InstructionCombining.cpp @@ -2956,9 +2956,26 @@ Instruction *InstCombiner::visitSDiv(BinaryOperator &I) { if (RHS->isAllOnesValue()) return BinaryOperator::CreateNeg(Op0); - // -X/C -> X/-C - if (Value *LHSNeg = dyn_castNegVal(Op0)) - return BinaryOperator::CreateSDiv(LHSNeg, ConstantExpr::getNeg(RHS)); + ConstantInt *RHSNeg = cast(ConstantExpr::getNeg(RHS)); + + // -X/C -> X/-C, if and only if negation doesn't overflow. + if ((RHS->getSExtValue() < 0 && + RHS->getSExtValue() < RHSNeg->getSExtValue()) || + (RHS->getSExtValue() > 0 && + RHS->getSExtValue() > RHSNeg->getSExtValue())) { + if (Value *LHSNeg = dyn_castNegVal(Op0)) { + if (ConstantInt *CI = dyn_cast(LHSNeg)) { + ConstantInt *CINeg = cast(ConstantExpr::getNeg(CI)); + + if ((CI->getSExtValue() < 0 && + CI->getSExtValue() < CINeg->getSExtValue()) || + (CI->getSExtValue() > 0 && + CI->getSExtValue() > CINeg->getSExtValue())) + return BinaryOperator::CreateSDiv(LHSNeg, + ConstantExpr::getNeg(RHS)); + } + } + } } // If the sign bits of both operands are zero (i.e. we can prove they are diff --git a/llvm/test/Transforms/InstCombine/apint-sub.ll b/llvm/test/Transforms/InstCombine/apint-sub.ll index 12f366de7f46..2ff763c9f5f8 100644 --- a/llvm/test/Transforms/InstCombine/apint-sub.ll +++ b/llvm/test/Transforms/InstCombine/apint-sub.ll @@ -3,7 +3,7 @@ ; ; RUN: llvm-as < %s | opt -instcombine | llvm-dis | \ -; RUN: grep -v {sub i19 %Cok, %Bok} | not grep sub +; RUN: grep -v {sub i19 %Cok, %Bok} | grep -v {sub i25 0, %Aok} | not grep sub ; END. define i23 @test1(i23 %A) { @@ -107,8 +107,10 @@ define i51 @test16(i51 %A) { ret i51 %Y } -define i25 @test17(i25 %A) { - %B = sub i25 0, %A ; [#uses=1] +; Can't fold subtract here because negation it might oveflow. +; PR3142 +define i25 @test17(i25 %Aok) { + %B = sub i25 0, %Aok ; [#uses=1] %C = sdiv i25 %B, 1234 ; [#uses=1] ret i25 %C } diff --git a/llvm/test/Transforms/InstCombine/sdiv-1.ll b/llvm/test/Transforms/InstCombine/sdiv-1.ll new file mode 100644 index 000000000000..c10bd775157a --- /dev/null +++ b/llvm/test/Transforms/InstCombine/sdiv-1.ll @@ -0,0 +1,22 @@ +; RUN: llvm-as < %s | opt -instcombine -inline | llvm-dis | grep {715827882} | count 2 +; PR3142 + +define i32 @a(i32 %X) nounwind readnone { +entry: + %0 = sub i32 0, %X + %1 = sdiv i32 %0, -3 + ret i32 %1 +} + +define i32 @b(i32 %X) nounwind readnone { +entry: + %0 = call i32 @a(i32 -2147483648) + ret i32 %0 +} + +define i32 @c(i32 %X) nounwind readnone { +entry: + %0 = sub i32 0, -2147483648 + %1 = sdiv i32 %0, -3 + ret i32 %1 +} diff --git a/llvm/test/Transforms/InstCombine/sub.ll b/llvm/test/Transforms/InstCombine/sub.ll index da6d710f6bc2..1ab4eaf1b49d 100644 --- a/llvm/test/Transforms/InstCombine/sub.ll +++ b/llvm/test/Transforms/InstCombine/sub.ll @@ -1,7 +1,7 @@ ; This test makes sure that these instructions are properly eliminated. ; ; RUN: llvm-as < %s | opt -instcombine | llvm-dis | \ -; RUN: grep -v {sub i32 %Cok, %Bok} | not grep sub +; RUN: grep -v {sub i32 %Cok, %Bok} | grep -v {sub i32 0, %Aok} | not grep sub define i32 @test1(i32 %A) { %B = sub i32 %A, %A ; [#uses=1] @@ -104,8 +104,10 @@ define i32 @test16(i32 %A) { ret i32 %Y } -define i32 @test17(i32 %A) { - %B = sub i32 0, %A ; [#uses=1] +; Can't fold subtract here because negation it might oveflow. +; PR3142 +define i32 @test17(i32 %Aok) { + %B = sub i32 0, %Aok ; [#uses=1] %C = sdiv i32 %B, 1234 ; [#uses=1] ret i32 %C } -- GitLab