[llvm] r221317 - Revert "[Reassociate] Canonicalize negative constants out of expressions."
Reid Kleckner
reid at kleckner.net
Tue Nov 4 15:42:45 PST 2014
Author: rnk
Date: Tue Nov 4 17:42:45 2014
New Revision: 221317
URL: http://llvm.org/viewvc/llvm-project?rev=221317&view=rev
Log:
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
Added:
llvm/trunk/test/Transforms/Reassociate/liftsign.ll
Removed:
llvm/trunk/test/Transforms/Reassociate/canonicalize-neg-const.ll
Modified:
llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp
llvm/trunk/test/Transforms/Reassociate/basictest.ll
Modified: llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp?rev=221317&r1=221316&r2=221317&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp Tue Nov 4 17:42:45 2014
@@ -193,8 +193,9 @@ namespace {
Value *OptimizeMul(BinaryOperator *I, SmallVectorImpl<ValueEntry> &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
}
}
-// 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<Constant>(I->getOperand(0));
- Constant *C1 = dyn_cast<Constant>(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<ConstantInt>(C)) {
- if (!CI->isNegative())
- return nullptr;
- } else if (auto *CF = dyn_cast<ConstantFP>(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<BinaryOperator>(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<ConstantInt>(C))
- I->setOperand(ConstIdx, ConstantInt::get(CI->getContext(), -CI->getValue()));
- else {
- ConstantFP *CF = cast<ConstantFP>(C);
- APFloat Val = CF->getValueAPF();
- Val.changeSign();
- I->setOperand(ConstIdx, ConstantFP::get(CF->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<BinaryOperator>(User)->swapOperands();
-
- 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<FPMathOperator>(User)->getFastMathFlags());
- break;
- case Instruction::FSub:
- NI = BinaryOperator::CreateFAdd(Op0, Op1);
- NI->setFastMathFlags(cast<FPMathOperator>(User)->getFastMathFlags());
- break;
- }
-
- NI->insertBefore(User);
- NI->setName(User->getName());
- User->replaceAllUsesWith(NI);
+ APFloat Val = ConstOperand->getValueAPF();
+ Val.changeSign();
+ I->setOperand(0, ConstantFP::get(ConstOperand->getContext(), Val));
+
+ assert(I->hasOneUse() && "Only a single use can be replaced.");
+ Instruction *Parent = I->user_back();
+
+ Value *OtherOperand = Parent->getOperand(1 - OperandNr);
+
+ 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<FPMathOperator>(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(Instructi
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(Instructi
}
}
+ // 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<ConstantFP>(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.
Modified: llvm/trunk/test/Transforms/Reassociate/basictest.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Reassociate/basictest.ll?rev=221317&r1=221316&r2=221317&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/Reassociate/basictest.ll (original)
+++ llvm/trunk/test/Transforms/Reassociate/basictest.ll Tue Nov 4 17:42:45 2014
@@ -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
}
Removed: llvm/trunk/test/Transforms/Reassociate/canonicalize-neg-const.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Reassociate/canonicalize-neg-const.ll?rev=221316&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/Reassociate/canonicalize-neg-const.ll (original)
+++ llvm/trunk/test/Transforms/Reassociate/canonicalize-neg-const.ll (removed)
@@ -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
-}
Added: llvm/trunk/test/Transforms/Reassociate/liftsign.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Reassociate/liftsign.ll?rev=221317&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/Reassociate/liftsign.ll (added)
+++ llvm/trunk/test/Transforms/Reassociate/liftsign.ll Tue Nov 4 17:42:45 2014
@@ -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
+}
More information about the llvm-commits
mailing list