From 094ac7735b375dfc34c413d4a2c4fd1ce30975ab Mon Sep 17 00:00:00 2001 From: Chad Rosier Date: Tue, 11 Nov 2014 22:58:35 +0000 Subject: [PATCH] [Reassociate] Canonicalize negative constants out of expressions. This is a reapplication of r221171, but we only perform the transformation on expressions which include a multiplication. We do not transform rem/div operations as this doesn't appear to be safe in all cases. llvm-svn: 221721 --- llvm/lib/Transforms/Scalar/Reassociate.cpp | 127 ++++++++++++------ llvm/test/Transforms/Reassociate/basictest.ll | 2 +- .../Reassociate/canonicalize-neg-const.ll | 110 +++++++++++++++ llvm/test/Transforms/Reassociate/liftsign.ll | 69 ---------- .../test/Transforms/Reassociate/wrap-flags.ll | 6 +- 5 files changed, 202 insertions(+), 112 deletions(-) create mode 100644 llvm/test/Transforms/Reassociate/canonicalize-neg-const.ll delete mode 100644 llvm/test/Transforms/Reassociate/liftsign.ll diff --git a/llvm/lib/Transforms/Scalar/Reassociate.cpp b/llvm/lib/Transforms/Scalar/Reassociate.cpp index 77e6a35119e0..7cde3ab6be8c 100644 --- a/llvm/lib/Transforms/Scalar/Reassociate.cpp +++ b/llvm/lib/Transforms/Scalar/Reassociate.cpp @@ -193,9 +193,8 @@ 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); }; } @@ -1947,31 +1946,95 @@ void Reassociate::EraseInst(Instruction *I) { } } -void Reassociate::optimizeFAddNegExpr(ConstantFP *ConstOperand, Instruction *I, - int OperandNr) { +// 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 be a mul instruction. + unsigned Opcode = I->getOpcode(); + if (Opcode != Instruction::Mul && Opcode != Instruction::FMul) + 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; + + 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; + // Change the sign of the constant. - APFloat Val = ConstOperand->getValueAPF(); - Val.changeSign(); - I->setOperand(0, ConstantFP::get(ConstOperand->getContext(), Val)); + 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)); + } - assert(I->hasOneUse() && "Only a single use can be replaced."); - Instruction *Parent = I->user_back(); + // 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(); - Value *OtherOperand = Parent->getOperand(1 - OperandNr); + 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; + } - 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->insertBefore(User); + NI->setName(User->getName()); + User->replaceAllUsesWith(NI); NI->setDebugLoc(I->getDebugLoc()); + RedoInsts.insert(I); MadeChange = true; + return NI; } /// OptimizeInst - Inspect and optimize the given instruction. Note that erasing @@ -1994,6 +2057,10 @@ 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. @@ -2014,24 +2081,6 @@ 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/llvm/test/Transforms/Reassociate/basictest.ll b/llvm/test/Transforms/Reassociate/basictest.ll index d70bfcbb72c6..0194ce2e302c 100644 --- a/llvm/test/Transforms/Reassociate/basictest.ll +++ b/llvm/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 %tmp, 47 +; CHECK-NEXT: mul i32 %B2, 47 ; CHECK-NEXT: ret i32 } diff --git a/llvm/test/Transforms/Reassociate/canonicalize-neg-const.ll b/llvm/test/Transforms/Reassociate/canonicalize-neg-const.ll new file mode 100644 index 000000000000..5ac8a1f26945 --- /dev/null +++ b/llvm/test/Transforms/Reassociate/canonicalize-neg-const.ll @@ -0,0 +1,110 @@ +; 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: 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 @test4(i64 %x, i64 %y) { +; CHECK-LABEL: @test4 +; 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 @test5(double %x, double %y) { +; CHECK-LABEL: @test5 +; 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 @test6(double %x, double %y) { +; CHECK-LABEL: @test6 +; 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 @test7(double %x, double %y) { +; CHECK-LABEL: @test7 +; 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 @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 %y, -1.234000e-01 + %add = fadd double %mul, %x + ret double %add +} diff --git a/llvm/test/Transforms/Reassociate/liftsign.ll b/llvm/test/Transforms/Reassociate/liftsign.ll deleted file mode 100644 index e8b728cba241..000000000000 --- a/llvm/test/Transforms/Reassociate/liftsign.ll +++ /dev/null @@ -1,69 +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) -; 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 -} diff --git a/llvm/test/Transforms/Reassociate/wrap-flags.ll b/llvm/test/Transforms/Reassociate/wrap-flags.ll index 7483acd02e51..e3304b6a7bb2 100644 --- a/llvm/test/Transforms/Reassociate/wrap-flags.ll +++ b/llvm/test/Transforms/Reassociate/wrap-flags.ll @@ -5,7 +5,7 @@ ; CHECK-LABEL: @shl_to_mul_nsw( ; CHECK: %mul = mul i32 %i, -2147483648 -; CHECK: %mul2 = add i32 %mul, 1 +; CHECK: add i32 %mul, 1 define i32 @shl_to_mul_nsw(i32 %i) { entry: %mul = shl nsw i32 %i, 31 @@ -15,7 +15,7 @@ entry: ; CHECK-LABEL: @shl_to_mul_nuw( ; CHECK: %mul = mul nuw i32 %i, 4 -; CHECK: %mul2 = add i32 %mul, 1 +; CHECK: add i32 %mul, 1 define i32 @shl_to_mul_nuw(i32 %i) { entry: %mul = shl nuw i32 %i, 2 @@ -25,7 +25,7 @@ entry: ; CHECK-LABEL: @shl_to_mul_nuw_nsw( ; CHECK: %mul = mul nuw nsw i32 %i, 4 -; CHECK: %mul2 = add i32 %mul, 1 +; CHECK: add i32 %mul, 1 define i32 @shl_to_mul_nuw_nsw(i32 %i) { entry: %mul = shl nuw nsw i32 %i, 2