From 11e6e005b7498b37d890f7c54ad1f13e06cb4e74 Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Tue, 4 Nov 2014 23:42:45 +0000 Subject: [PATCH] Revert "[Reassociate] Canonicalize negative constants out of expressions." This reverts commit r221171. It performs this invalid transformation: - %div.i = urem i64 -1, %add - %sub.i = sub i64 -2, %div.i + %div.i = urem i64 1, %add + %sub.i1 = add i64 %div.i, -2 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@221317 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Scalar/Reassociate.cpp | 137 +++------- test/Transforms/Reassociate/basictest.ll | 2 +- .../Reassociate/canonicalize-neg-const.ll | 235 ------------------ test/Transforms/Reassociate/liftsign.ll | 69 +++++ 4 files changed, 109 insertions(+), 334 deletions(-) delete mode 100644 test/Transforms/Reassociate/canonicalize-neg-const.ll create mode 100644 test/Transforms/Reassociate/liftsign.ll diff --git a/lib/Transforms/Scalar/Reassociate.cpp b/lib/Transforms/Scalar/Reassociate.cpp index a9d6c89a35c..0d5ad7311fb 100644 --- a/lib/Transforms/Scalar/Reassociate.cpp +++ b/lib/Transforms/Scalar/Reassociate.cpp @@ -193,8 +193,9 @@ namespace { Value *OptimizeMul(BinaryOperator *I, SmallVectorImpl &Ops); Value *RemoveFactorFromExpression(Value *V, Value *Factor); void EraseInst(Instruction *I); + void optimizeFAddNegExpr(ConstantFP *ConstOperand, Instruction *I, + int OperandNr); void OptimizeInst(Instruction *I); - Instruction *canonicalizeNegConstExpr(Instruction *I); }; } @@ -1929,105 +1930,31 @@ void Reassociate::EraseInst(Instruction *I) { } } -// Canonicalize expressions of the following form: -// x + (-Constant * y) -> x - (Constant * y) -// x - (-Constant * y) -> x + (Constant * y) -Instruction *Reassociate::canonicalizeNegConstExpr(Instruction *I) { - if (!I->hasOneUse() || I->getType()->isVectorTy()) - return nullptr; - - // Must have at least one constant operand. - Constant *C0 = dyn_cast(I->getOperand(0)); - Constant *C1 = dyn_cast(I->getOperand(1)); - if (!C0 && !C1) - return nullptr; - - // Must be a negative ConstantInt or ConstantFP. - Constant *C = C0 ? C0 : C1; - unsigned ConstIdx = C0 ? 0 : 1; - if (auto *CI = dyn_cast(C)) { - if (!CI->isNegative()) - return nullptr; - } else if (auto *CF = dyn_cast(C)) { - if (!CF->isNegative()) - return nullptr; - } else - return nullptr; - - // User must be a binary operator with one or more uses. - Instruction *User = I->user_back(); - if (!isa(User) || !User->getNumUses()) - return nullptr; - - // Must be a binary operator with higher precedence that add/sub. - switch(I->getOpcode()) { - default: - return nullptr; - case Instruction::Mul: - case Instruction::FMul: - case Instruction::UDiv: - case Instruction::SDiv: - case Instruction::FDiv: - case Instruction::URem: - case Instruction::SRem: - case Instruction::FRem: - break; - } - - unsigned UserOpcode = User->getOpcode(); - if (UserOpcode != Instruction::Add && UserOpcode != Instruction::FAdd && - UserOpcode != Instruction::Sub && UserOpcode != Instruction::FSub) - return nullptr; - - // Subtraction is not commutative. Explicitly, the following transform is - // not valid: (-Constant * y) - x -> x + (Constant * y) - if (!User->isCommutative() && User->getOperand(1) != I) - return nullptr; - +void Reassociate::optimizeFAddNegExpr(ConstantFP *ConstOperand, Instruction *I, + int OperandNr) { // Change the sign of the constant. - if (ConstantInt *CI = dyn_cast(C)) - I->setOperand(ConstIdx, ConstantInt::get(CI->getContext(), -CI->getValue())); - else { - ConstantFP *CF = cast(C); - APFloat Val = CF->getValueAPF(); - Val.changeSign(); - I->setOperand(ConstIdx, ConstantFP::get(CF->getContext(), Val)); - } + APFloat Val = ConstOperand->getValueAPF(); + Val.changeSign(); + I->setOperand(0, ConstantFP::get(ConstOperand->getContext(), Val)); - // Canonicalize I to RHS to simplify the next bit of logic. E.g., - // ((-Const*y) + x) -> (x + (-Const*y)). - if (User->getOperand(0) == I && User->isCommutative()) - cast(User)->swapOperands(); + assert(I->hasOneUse() && "Only a single use can be replaced."); + Instruction *Parent = I->user_back(); - Value *Op0 = User->getOperand(0); - Value *Op1 = User->getOperand(1); - BinaryOperator *NI; - switch(UserOpcode) { - default: - llvm_unreachable("Unexpected Opcode!"); - case Instruction::Add: - NI = BinaryOperator::CreateSub(Op0, Op1); - break; - case Instruction::Sub: - NI = BinaryOperator::CreateAdd(Op0, Op1); - break; - case Instruction::FAdd: - NI = BinaryOperator::CreateFSub(Op0, Op1); - NI->setFastMathFlags(cast(User)->getFastMathFlags()); - break; - case Instruction::FSub: - NI = BinaryOperator::CreateFAdd(Op0, Op1); - NI->setFastMathFlags(cast(User)->getFastMathFlags()); - break; - } + Value *OtherOperand = Parent->getOperand(1 - OperandNr); - NI->insertBefore(User); - NI->setName(User->getName()); - User->replaceAllUsesWith(NI); + unsigned Opcode = Parent->getOpcode(); + assert(Opcode == Instruction::FAdd || + (Opcode == Instruction::FSub && Parent->getOperand(1) == I)); + + BinaryOperator *NI = Opcode == Instruction::FAdd + ? BinaryOperator::CreateFSub(OtherOperand, I) + : BinaryOperator::CreateFAdd(OtherOperand, I); + NI->setFastMathFlags(cast(Parent)->getFastMathFlags()); + NI->insertBefore(Parent); + NI->setName(Parent->getName() + ".repl"); + Parent->replaceAllUsesWith(NI); NI->setDebugLoc(I->getDebugLoc()); - RedoInsts.insert(I); MadeChange = true; - return NI; } /// OptimizeInst - Inspect and optimize the given instruction. Note that erasing @@ -2050,10 +1977,6 @@ void Reassociate::OptimizeInst(Instruction *I) { I = NI; } - // Canonicalize negative constants out of expressions. - if (Instruction *Res = canonicalizeNegConstExpr(I)) - I = Res; - // Commute floating point binary operators, to canonicalize the order of their // operands. This can potentially expose more CSE opportunities, and makes // writing other transformations simpler. @@ -2074,6 +1997,24 @@ void Reassociate::OptimizeInst(Instruction *I) { } } + // Reassociate: x + -ConstantFP * y -> x - ConstantFP * y + // The FMul can also be an FDiv, and FAdd can be a FSub. + if (Opcode == Instruction::FMul || Opcode == Instruction::FDiv) { + if (ConstantFP *LHSConst = dyn_cast(I->getOperand(0))) { + if (LHSConst->isNegative() && I->hasOneUse()) { + Instruction *Parent = I->user_back(); + if (Parent->getOpcode() == Instruction::FAdd) { + if (Parent->getOperand(0) == I) + optimizeFAddNegExpr(LHSConst, I, 0); + else if (Parent->getOperand(1) == I) + optimizeFAddNegExpr(LHSConst, I, 1); + } else if (Parent->getOpcode() == Instruction::FSub) + if (Parent->getOperand(1) == I) + optimizeFAddNegExpr(LHSConst, I, 1); + } + } + } + // FIXME: We should commute vector instructions as well. However, this // requires further analysis to determine the effect on later passes. diff --git a/test/Transforms/Reassociate/basictest.ll b/test/Transforms/Reassociate/basictest.ll index 0194ce2e302..d70bfcbb72c 100644 --- a/test/Transforms/Reassociate/basictest.ll +++ b/test/Transforms/Reassociate/basictest.ll @@ -203,7 +203,7 @@ define i32 @test14(i32 %X1, i32 %X2) { ; CHECK-LABEL: @test14 ; CHECK-NEXT: sub i32 %X1, %X2 -; CHECK-NEXT: mul i32 %B2, 47 +; CHECK-NEXT: mul i32 %tmp, 47 ; CHECK-NEXT: ret i32 } diff --git a/test/Transforms/Reassociate/canonicalize-neg-const.ll b/test/Transforms/Reassociate/canonicalize-neg-const.ll deleted file mode 100644 index 4001bbcfafc..00000000000 --- a/test/Transforms/Reassociate/canonicalize-neg-const.ll +++ /dev/null @@ -1,235 +0,0 @@ -; RUN: opt -reassociate -gvn -S < %s | FileCheck %s - -; (x + 0.1234 * y) * (x + -0.1234 * y) -> (x + 0.1234 * y) * (x - 0.1234 * y) -define double @test1(double %x, double %y) { -; CHECK-LABEL: @test1 -; CHECK-NEXT: fmul double 1.234000e-01, %y -; CHECK-NEXT: fadd double %x, %mul -; CHECK-NEXT: fsub double %x, %mul -; CHECK-NEXT: fmul double %add{{.*}}, %add{{.*}} -; CHECK-NEXT: ret double %mul - - %mul = fmul double 1.234000e-01, %y - %add = fadd double %mul, %x - %mul1 = fmul double -1.234000e-01, %y - %add2 = fadd double %mul1, %x - %mul3 = fmul double %add, %add2 - ret double %mul3 -} - -; (x + -0.1234 * y) * (x + -0.1234 * y) -> (x - 0.1234 * y) * (x - 0.1234 * y) -define double @test2(double %x, double %y) { -; CHECK-LABEL: @test2 -; CHECK-NEXT: fmul double 1.234000e-01, %y -; CHECK-NEXT: fsub double %x, %mul -; CHECK-NEXT: fmul double %add{{.*}}, %add{{.*}} -; CHECK-NEXT: ret double %mul - - %mul = fmul double %y, -1.234000e-01 - %add = fadd double %mul, %x - %mul1 = fmul double %y, -1.234000e-01 - %add2 = fadd double %mul1, %x - %mul3 = fmul double %add, %add2 - ret double %mul3 -} - -; (x + 0.1234 / y) * (x + -0.1234 / y) -> (x + 0.1234 / y) * (x - 0.1234 / y) -define double @test3(double %x, double %y) { -; CHECK-LABEL: @test3 -; CHECK-NEXT: fdiv double 1.234000e-01, %y -; CHECK-NEXT: fadd double %x, %div -; CHECK-NEXT: fsub double %x, %div -; CHECK-NEXT: fmul double %add{{.*}}, %add{{.*}} -; CHECK-NEXT: ret double - - %div = fdiv double 1.234000e-01, %y - %add = fadd double %div, %x - %div1 = fdiv double -1.234000e-01, %y - %add2 = fadd double %div1, %x - %mul3 = fmul double %add, %add2 - ret double %mul3 -} - -; (x + 0.1234 * y) * (x - -0.1234 * y) -> (x + 0.1234 * y) * (x + 0.1234 * y) -define double @test4(double %x, double %y) { -; CHECK-LABEL: @test4 -; CHECK-NEXT: fmul double 1.234000e-01, %y -; CHECK-NEXT: fadd double %x, %mul -; CHECK-NEXT: fmul double %add{{.*}}, %add{{.*}} -; CHECK-NEXT: ret double - - %mul = fmul double %y, 1.234000e-01 - %add = fadd double %mul, %x - %mul1 = fmul double %y, -1.234000e-01 - %add2 = fsub double %x, %mul1 - %mul3 = fmul double %add, %add2 - ret double %mul3 -} - -; Canonicalize (x - -1234 * y) -define i64 @test5(i64 %x, i64 %y) { -; CHECK-LABEL: @test5 -; CHECK-NEXT: mul i64 %y, 1234 -; CHECK-NEXT: add i64 %mul, %x -; CHECK-NEXT: ret i64 %sub - - %mul = mul i64 %y, -1234 - %sub = sub i64 %x, %mul - ret i64 %sub -} - -; Canonicalize (x - -0.1234 * y) -define double @test6(double %x, double %y) { -; CHECK-LABEL: @test6 -; CHECK-NEXT: fmul double 1.234000e-01, %y -; CHECK-NEXT: fadd double %x, %mul -; CHECK-NEXT: ret double - - %mul = fmul double -1.234000e-01, %y - %sub = fsub double %x, %mul - ret double %sub -} - -; Don't modify (-0.1234 * y - x) -define double @test7(double %x, double %y) { -; CHECK-LABEL: @test7 -; CHECK-NEXT: fmul double -1.234000e-01, %y -; CHECK-NEXT: fsub double %mul, %x -; CHECK-NEXT: ret double %sub - - %mul = fmul double -1.234000e-01, %y - %sub = fsub double %mul, %x - ret double %sub -} - -; Canonicalize (-0.1234 * y + x) -> (x - 0.1234 * y) -define double @test8(double %x, double %y) { -; CHECK-LABEL: @test8 -; CHECK-NEXT: fmul double 1.234000e-01, %y -; CHECK-NEXT: fsub double %x, %mul -; CHECK-NEXT: ret double %add - - %mul = fmul double -1.234000e-01, %y - %add = fadd double %mul, %x - ret double %add -} - -; Canonicalize (y * -0.1234 + x) -> (x - 0.1234 * y) -define double @test9(double %x, double %y) { -; CHECK-LABEL: @test9 -; CHECK-NEXT: fmul double 1.234000e-01, %y -; CHECK-NEXT: fsub double %x, %mul -; CHECK-NEXT: ret double %add - - %mul = fmul double %y, -1.234000e-01 - %add = fadd double %mul, %x - ret double %add -} - -; Canonicalize (x - -1234 udiv y) -define i64 @test10(i64 %x, i64 %y) { -; CHECK-LABEL: @test10 -; CHECK-NEXT: udiv i64 1234, %y -; CHECK-NEXT: add i64 %div, %x -; CHECK-NEXT: ret i64 %sub - - %div = udiv i64 -1234, %y - %sub = sub i64 %x, %div - ret i64 %sub -} - -; Canonicalize (x - -1234 sdiv y) -define i64 @test11(i64 %x, i64 %y) { -; CHECK-LABEL: @test11 -; CHECK-NEXT: sdiv i64 1234, %y -; CHECK-NEXT: add i64 %div, %x -; CHECK-NEXT: ret i64 %sub - - %div = sdiv i64 -1234, %y - %sub = sub i64 %x, %div - ret i64 %sub -} - -; Canonicalize (x - -0.1234 / y) -define double @test12(double %x, double %y) { -; CHECK-LABEL: @test12 -; CHECK-NEXT: fdiv double 1.234000e-01, %y -; CHECK-NEXT: fadd double %x, %div -; CHECK-NEXT: ret double %sub - - %div = fdiv double -1.234000e-01, %y - %sub = fsub double %x, %div - ret double %sub -} - -; Canonicalize (x - -0.1234 urem y) -define i64 @test13(i64 %x, i64 %y) { -; CHECK-LABEL: @test13 -; CHECK-NEXT: urem i64 1234, %y -; CHECK-NEXT: add i64 %urem, %x -; CHECK-NEXT: ret i64 %sub - - %urem = urem i64 -1234, %y - %sub = sub i64 %x, %urem - ret i64 %sub -} - -; Canonicalize (x + -0.1234 srem y) -define i64 @test14(i64 %x, i64 %y) { -; CHECK-LABEL: @test14 -; CHECK-NEXT: srem i64 1234, %y -; CHECK-NEXT: sub i64 %x, %srem -; CHECK-NEXT: ret i64 %add - - %srem = srem i64 -1234, %y - %add = add i64 %x, %srem - ret i64 %add -} - -; Don't modify (-0.1234 srem y - x) -define i64 @test15(i64 %x, i64 %y) { -; CHECK-LABEL: @test15 -; CHECK-NEXT: %srem = srem i64 -1234, %y -; CHECK-NEXT: %sub = sub i64 %srem, %x -; CHECK-NEXT: ret i64 %sub - - %srem = srem i64 -1234, %y - %sub = sub i64 %srem, %x - ret i64 %sub -} - -; Canonicalize (x - -0.1234 frem y) -define double @test16(double %x, double %y) { -; CHECK-LABEL: @test16 -; CHECK-NEXT: frem double 1.234000e-01, %y -; CHECK-NEXT: fadd double %x, %rem -; CHECK-NEXT: ret double %sub - - %rem = frem double -1.234000e-01, %y - %sub = fsub double %x, %rem - ret double %sub -} - -; Canonicalize (x + -0.1234 frem y) -define double @test17(double %x, double %y) { -; CHECK-LABEL: @test17 -; CHECK-NEXT: frem double 1.234000e-01, %y -; CHECK-NEXT: fsub double %x, %rem -; CHECK-NEXT: ret double %add - - %rem = frem double -1.234000e-01, %y - %add = fadd double %x, %rem - ret double %add -} - -; Canonicalize (-0.1234 frem y + x) -> (x - 0.1234 frem y) -define double @test18(double %x, double %y) { -; CHECK-LABEL: @test18 -; CHECK-NEXT: frem double 1.234000e-01, %y -; CHECK-NEXT: fsub double %x, %rem -; CHECK-NEXT: ret double %add - - %rem = frem double -1.234000e-01, %y - %add = fadd double %x, %rem - ret double %add -} diff --git a/test/Transforms/Reassociate/liftsign.ll b/test/Transforms/Reassociate/liftsign.ll new file mode 100644 index 00000000000..e8b728cba24 --- /dev/null +++ b/test/Transforms/Reassociate/liftsign.ll @@ -0,0 +1,69 @@ +; RUN: opt -reassociate -gvn -S < %s | FileCheck %s + +; (x + 0.1234 * y) * (x + -0.1234 * y) -> (x + 0.1234 * y) * (x - 0.1234 * y) +; so CSE can simplify it further +define double @lift_sign1(double %x, double %y) nounwind readnone ssp uwtable { +; CHECK-LABEL: @lift_sign1( + %mul = fmul double 1.234000e-01, %y + %add = fadd double %mul, %x + %mul1 = fmul double -1.234000e-01, %y + %add2 = fadd double %mul1, %x + %mul3 = fmul double %add, %add2 +; CHECK-NOT: %mul1 = fmul double -1.234000e-01, %y +; CHECK-NOT: %add2 = fadd %mul1, %x +; CHECK: %add2.repl = fsub double %x, %mul +; CHECK: %mul3 = fmul double %add, %add2 +ret double %mul3 +} + +; (x + -0.1234 * y) * (x + -0.1234 * y) -> (x - 0.1234 * y) * (x - 0.1234 * y) +; GVN can then rewrite it even further +define double @lift_sign2(double %x, double %y) nounwind readnone ssp uwtable { +; CHECK-LABEL: @lift_sign2( + %mul = fmul double %y, -1.234000e-01 + %add = fadd double %mul, %x + %mul1 = fmul double %y, -1.234000e-01 + %add2 = fadd double %mul1, %x + %mul3 = fmul double %add, %add2 +; CHECK-NOT: %mul = fmul double %y, -1.234000e-01 +; CHECK-NOT: %add = fadd double %mul, %x +; CHECK-NOT: %mul1 = fmul double %y, -1.234000e-01 +; CHECK-NOT: %add2 = fadd double %mul1, %x +; CHECK-NOT: %mul3 = fmul double %add, %add2 +; CHECK: %mul = fmul double 1.234000e-01, %y +; CHECK: %add.repl = fsub double %x, %mul +; CHECK: %mul3 = fmul double %add.repl, %add.repl + ret double %mul3 +} + +; (x + 0.1234 * y) * (x - -0.1234 * y) -> (x + 0.1234 * y) * (x + 0.1234 * y) +define double @lift_sign3(double %x, double %y) nounwind readnone ssp uwtable { +; CHECK-LABEL: @lift_sign3( + %mul = fmul double %y, 1.234000e-01 + %add = fadd double %mul, %x + %mul1 = fmul double %y, -1.234000e-01 + %add2 = fsub double %x, %mul1 + %mul3 = fmul double %add, %add2 +; CHECK-NOT: %mul1 = fmul double %y, -1.234000e-01 +; CHECK-NOT: %add2 = fsub double %x, %mul1 +; CHECK-NOT: %mul3 = fmul double %add, %add2 +; CHECK: %mul3 = fmul double %add, %add + ret double %mul3 +} + +; (x + 0.1234 / y) * (x + -0.1234 / y) -> (x + 0.1234 / y) * (x - 0.1234 / y) +; so CSE can simplify it further +define double @lift_sign4(double %x, double %y) nounwind readnone ssp uwtable { +; CHECK-LABEL: @lift_sign4( + %div = fdiv double 1.234000e-01, %y + %add = fadd double %div, %x + %div1 = fdiv double -1.234000e-01, %y + %add2 = fadd double %div1, %x + %mul3 = fmul double %add, %add2 +; CHECK-NOT: %div1 = fdiv double -1.234000e-01, %y +; CHECK-NOT: %add2 = fadd double %div1, %x +; CHECK-NOT: %mul3 = fmul double %add, %add2 +; CHECK: %add2.repl = fsub double %x, %div +; CHECK: %mul3 = fmul double %add, %add2.repl + ret double %mul3 +}