From 16395e51f4cc25748bf4f6b9f21549680b43c620 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Mon, 14 Jul 2008 00:15:52 +0000 Subject: [PATCH] Fix PR2506 by being a bit more careful about reverse fact propagation when disproving a condition. This actually compiles the existing testcase (udiv_select_to_select_shift) to: define i64 @test(i64 %X, i1 %Cond) { entry: %divisor1.t = lshr i64 %X, 3 ; [#uses=1] %quotient2 = lshr i64 %X, 3 ; [#uses=1] %sum = add i64 %divisor1.t, %quotient2 ; [#uses=1] ret i64 %sum } instead of: define i64 @test(i64 %X, i1 %Cond) { entry: %quotient1.v = select i1 %Cond, i64 3, i64 4 ; [#uses=1] %quotient1 = lshr i64 %X, %quotient1.v ; [#uses=1] %quotient2 = lshr i64 %X, 3 ; [#uses=1] %sum = add i64 %quotient1, %quotient2 ; [#uses=1] ret i64 %sum } llvm-svn: 53534 --- .../Scalar/InstructionCombining.cpp | 144 ++++++++++-------- .../InstCombine/2008-07-13-DivZero.ll | 16 ++ .../udiv_select_to_select_shift.ll | 6 +- 3 files changed, 99 insertions(+), 67 deletions(-) create mode 100644 llvm/test/Transforms/InstCombine/2008-07-13-DivZero.ll diff --git a/llvm/lib/Transforms/Scalar/InstructionCombining.cpp b/llvm/lib/Transforms/Scalar/InstructionCombining.cpp index 20580e30ab47..743c0c1a302f 100644 --- a/llvm/lib/Transforms/Scalar/InstructionCombining.cpp +++ b/llvm/lib/Transforms/Scalar/InstructionCombining.cpp @@ -172,6 +172,7 @@ namespace { Instruction *visitURem(BinaryOperator &I); Instruction *visitSRem(BinaryOperator &I); Instruction *visitFRem(BinaryOperator &I); + bool SimplifyDivRemOfSelect(BinaryOperator &I); Instruction *commonRemTransforms(BinaryOperator &I); Instruction *commonIRemTransforms(BinaryOperator &I); Instruction *commonDivTransforms(BinaryOperator &I); @@ -2566,6 +2567,78 @@ Instruction *InstCombiner::visitMul(BinaryOperator &I) { return Changed ? &I : 0; } +/// SimplifyDivRemOfSelect - Try to fold a divide or remainder of a select +/// instruction. +bool InstCombiner::SimplifyDivRemOfSelect(BinaryOperator &I) { + SelectInst *SI = cast(I.getOperand(1)); + + // div/rem X, (Cond ? 0 : Y) -> div/rem X, Y + int NonNullOperand = -1; + if (Constant *ST = dyn_cast(SI->getOperand(1))) + if (ST->isNullValue()) + NonNullOperand = 2; + // div/rem X, (Cond ? Y : 0) -> div/rem X, Y + if (Constant *ST = dyn_cast(SI->getOperand(2))) + if (ST->isNullValue()) + NonNullOperand = 1; + + if (NonNullOperand == -1) + return false; + + Value *SelectCond = SI->getOperand(0); + + // Change the div/rem to use 'Y' instead of the select. + I.setOperand(1, SI->getOperand(NonNullOperand)); + + // Okay, we know we replace the operand of the div/rem with 'Y' with no + // problem. However, the select, or the condition of the select may have + // multiple uses. Based on our knowledge that the operand must be non-zero, + // propagate the known value for the select into other uses of it, and + // propagate a known value of the condition into its other users. + + // If the select and condition only have a single use, don't bother with this, + // early exit. + if (SI->use_empty() && SelectCond->hasOneUse()) + return true; + + // Scan the current block backward, looking for other uses of SI. + BasicBlock::iterator BBI = &I, BBFront = I.getParent()->begin(); + + while (BBI != BBFront) { + --BBI; + // If we found a call to a function, we can't assume it will return, so + // information from below it cannot be propagated above it. + if (isa(BBI) && !isa(BBI)) + break; + + // Replace uses of the select or its condition with the known values. + for (Instruction::op_iterator I = BBI->op_begin(), E = BBI->op_end(); + I != E; ++I) { + if (*I == SI) { + *I = SI->getOperand(NonNullOperand); + AddToWorkList(BBI); + } else if (*I == SelectCond) { + *I = NonNullOperand == 1 ? ConstantInt::getTrue() : + ConstantInt::getFalse(); + AddToWorkList(BBI); + } + } + + // If we past the instruction, quit looking for it. + if (&*BBI == SI) + SI = 0; + if (&*BBI == SelectCond) + SelectCond = 0; + + // If we ran out of things to eliminate, break out of the loop. + if (SelectCond == 0 && SI == 0) + break; + + } + return true; +} + + /// This function implements the transforms on div instructions that work /// regardless of the kind of div instruction it is (udiv, sdiv, or fdiv). It is /// used by the visitors to those instructions. @@ -2585,40 +2658,6 @@ Instruction *InstCombiner::commonDivTransforms(BinaryOperator &I) { if (isa(Op1)) return ReplaceInstUsesWith(I, Op1); - // Handle cases involving: [su]div X, (select Cond, Y, Z) - // This does not apply for fdiv. - if (SelectInst *SI = dyn_cast(Op1)) { - // [su]div X, (Cond ? 0 : Y) -> div X, Y. If the div and the select are in - // the same basic block, then we replace the select with Y, and the - // condition of the select with false (if the cond value is in the same BB). - // If the select has uses other than the div, this allows them to be - // simplified also. Note that div X, Y is just as good as div X, 0 (undef) - if (ConstantInt *ST = dyn_cast(SI->getOperand(1))) - if (ST->isNullValue()) { - Instruction *CondI = dyn_cast(SI->getOperand(0)); - if (CondI && CondI->getParent() == I.getParent()) - UpdateValueUsesWith(CondI, ConstantInt::getFalse()); - else if (I.getParent() != SI->getParent() || SI->hasOneUse()) - I.setOperand(1, SI->getOperand(2)); - else - UpdateValueUsesWith(SI, SI->getOperand(2)); - return &I; - } - - // Likewise for: [su]div X, (Cond ? Y : 0) -> div X, Y - if (ConstantInt *ST = dyn_cast(SI->getOperand(2))) - if (ST->isNullValue()) { - Instruction *CondI = dyn_cast(SI->getOperand(0)); - if (CondI && CondI->getParent() == I.getParent()) - UpdateValueUsesWith(CondI, ConstantInt::getTrue()); - else if (I.getParent() != SI->getParent() || SI->hasOneUse()) - I.setOperand(1, SI->getOperand(1)); - else - UpdateValueUsesWith(SI, SI->getOperand(1)); - return &I; - } - } - return 0; } @@ -2643,6 +2682,11 @@ Instruction *InstCombiner::commonIDivTransforms(BinaryOperator &I) { if (Instruction *Common = commonDivTransforms(I)) return Common; + + // Handle cases involving: [su]div X, (select Cond, Y, Z) + // This does not apply for fdiv. + if (isa(Op1) && SimplifyDivRemOfSelect(I)) + return &I; if (ConstantInt *RHS = dyn_cast(Op1)) { // div X, 1 == X @@ -2798,36 +2842,8 @@ Instruction *InstCombiner::commonRemTransforms(BinaryOperator &I) { return ReplaceInstUsesWith(I, Op1); // X % undef -> undef // Handle cases involving: rem X, (select Cond, Y, Z) - if (SelectInst *SI = dyn_cast(Op1)) { - // rem X, (Cond ? 0 : Y) -> rem X, Y. If the rem and the select are in - // the same basic block, then we replace the select with Y, and the - // condition of the select with false (if the cond value is in the same - // BB). If the select has uses other than the div, this allows them to be - // simplified also. - if (Constant *ST = dyn_cast(SI->getOperand(1))) - if (ST->isNullValue()) { - Instruction *CondI = dyn_cast(SI->getOperand(0)); - if (CondI && CondI->getParent() == I.getParent()) - UpdateValueUsesWith(CondI, ConstantInt::getFalse()); - else if (I.getParent() != SI->getParent() || SI->hasOneUse()) - I.setOperand(1, SI->getOperand(2)); - else - UpdateValueUsesWith(SI, SI->getOperand(2)); - return &I; - } - // Likewise for: rem X, (Cond ? Y : 0) -> rem X, Y - if (Constant *ST = dyn_cast(SI->getOperand(2))) - if (ST->isNullValue()) { - Instruction *CondI = dyn_cast(SI->getOperand(0)); - if (CondI && CondI->getParent() == I.getParent()) - UpdateValueUsesWith(CondI, ConstantInt::getTrue()); - else if (I.getParent() != SI->getParent() || SI->hasOneUse()) - I.setOperand(1, SI->getOperand(1)); - else - UpdateValueUsesWith(SI, SI->getOperand(1)); - return &I; - } - } + if (isa(Op1) && SimplifyDivRemOfSelect(I)) + return &I; return 0; } diff --git a/llvm/test/Transforms/InstCombine/2008-07-13-DivZero.ll b/llvm/test/Transforms/InstCombine/2008-07-13-DivZero.ll new file mode 100644 index 000000000000..85c3dbc5ff69 --- /dev/null +++ b/llvm/test/Transforms/InstCombine/2008-07-13-DivZero.ll @@ -0,0 +1,16 @@ +; RUN: llvm-as < %s | opt -instcombine | llvm-dis | grep {lshr.*3} +; RUN: llvm-as < %s | opt -instcombine | llvm-dis | grep {call .*%cond} +; PR2506 + +; We can simplify the operand of udiv to '8', but not the operand to the +; call. If the callee never returns, we can't assume the div is reachable. +define i32 @a(i32 %x, i32 %y) { +entry: + %tobool = icmp ne i32 %y, 0 ; [#uses=1] + %cond = select i1 %tobool, i32 8, i32 0 ; [#uses=2] + %call = call i32 @b( i32 %cond ) ; [#uses=0] + %div = udiv i32 %x, %cond ; [#uses=1] + ret i32 %div +} + +declare i32 @b(i32) diff --git a/llvm/test/Transforms/InstCombine/udiv_select_to_select_shift.ll b/llvm/test/Transforms/InstCombine/udiv_select_to_select_shift.ll index 277592c983da..614ae3dc975c 100644 --- a/llvm/test/Transforms/InstCombine/udiv_select_to_select_shift.ll +++ b/llvm/test/Transforms/InstCombine/udiv_select_to_select_shift.ll @@ -2,13 +2,13 @@ ; udiv X, (Select Cond, C1, C2) --> Select Cond, (shr X, C1), (shr X, C2) ; ; RUN: llvm-as < %s | opt -instcombine | llvm-dis -f -o %t -; RUN: grep select %t | count 1 +; RUN: not grep select %t ; RUN: grep lshr %t | count 2 -; RUN: ignore grep udiv %t | count 0 +; RUN: not grep udiv %t define i64 @test(i64 %X, i1 %Cond ) { entry: - %divisor1 = select i1 %Cond, i64 8, i64 16 + %divisor1 = select i1 %Cond, i64 16, i64 8 %quotient1 = udiv i64 %X, %divisor1 %divisor2 = select i1 %Cond, i64 8, i64 0 %quotient2 = udiv i64 %X, %divisor2