[PATCH][DAGCombine] Fix bug in DAGCombine
Shuxin Yang
shuxin.llvm at gmail.com
Fri Feb 1 16:09:34 PST 2013
Hi,
rdar://13126763.
The attached patch fixes an inadvertent bug in DAG-combine. This
bug was
introduced by r162956 back to Aug 2012. The symptom is to mistakenly
optimize "x + x*x" into "x * 3.0". Following pseudo-code illustrate
the mistaken
logic. The fix is quite straightforward.
-----------------------
DAGCombiner::visitFADD(SDNode *N) {
Let N0, N1 be the 1st and 2nd operand of N;
if (N0 is fmul) {
...
!!!! The sub-expr must be fmul, Yuck !!!!!
// (fadd x, (fadd x, x)) -> (fmul 3.0, x)
}
-----------------------------
Thanks
Shuxin
-------------- next part --------------
Index: test/CodeGen/X86/dagcombine_unsafe_math.ll
===================================================================
--- test/CodeGen/X86/dagcombine_unsafe_math.ll (revision 0)
+++ test/CodeGen/X86/dagcombine_unsafe_math.ll (revision 0)
@@ -0,0 +1,42 @@
+; RUN: llc < %s -enable-unsafe-fp-math -mtriple=x86_64-apple-darwin -mcpu=corei7-avx | FileCheck %s
+
+
+; rdar://13126763
+; Expression "x + x*x" was mistakenly transformed into "x * 3.0f".
+
+define float @test1(float %x) {
+ %t1 = fmul fast float %x, %x
+ %t2 = fadd fast float %t1, %x
+ ret float %t2
+; CHECK: test1
+; CHECK: vaddss
+}
+
+; (x + x) + x => x * 3.0
+define float @test2(float %x) {
+ %t1 = fadd fast float %x, %x
+ %t2 = fadd fast float %t1, %x
+ ret float %t2
+; CHECK: .long 1077936128
+; CHECK: test2
+; CHECK: vmulss LCPI1_0(%rip), %xmm0, %xmm0
+}
+
+; x + (x + x) => x * 3.0
+define float @test3(float %x) {
+ %t1 = fadd fast float %x, %x
+ %t2 = fadd fast float %t1, %x
+ ret float %t2
+; CHECK: .long 1077936128
+; CHECK: test3
+; CHECK: vmulss LCPI2_0(%rip), %xmm0, %xmm0
+}
+
+; (y + x) + x != x * 3.0
+define float @test4(float %x, float %y) {
+ %t1 = fadd fast float %x, %y
+ %t2 = fadd fast float %t1, %x
+ ret float %t2
+; CHECK: test4
+; CHECK: vaddss
+}
Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/DAGCombiner.cpp (revision 174228)
+++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp (working copy)
@@ -5832,13 +5832,6 @@
N1, NewCFP);
}
- // (fadd (fadd x, x), x) -> (fmul 3.0, x)
- if (!CFP00 && !CFP01 && N0.getOperand(0) == N0.getOperand(1) &&
- N0.getOperand(0) == N1) {
- return DAG.getNode(ISD::FMUL, N->getDebugLoc(), VT,
- N1, DAG.getConstantFP(3.0, VT));
- }
-
// (fadd (fmul c, x), (fadd x, x)) -> (fmul c+2, x)
if (CFP00 && !CFP01 && N1.getOpcode() == ISD::FADD &&
N1.getOperand(0) == N1.getOperand(1) &&
@@ -5884,12 +5877,6 @@
N0, NewCFP);
}
- // (fadd x, (fadd x, x)) -> (fmul 3.0, x)
- if (!CFP10 && !CFP11 && N1.getOperand(0) == N1.getOperand(1) &&
- N1.getOperand(0) == N0) {
- return DAG.getNode(ISD::FMUL, N->getDebugLoc(), VT,
- N0, DAG.getConstantFP(3.0, VT));
- }
// (fadd (fadd x, x), (fmul c, x)) -> (fmul c+2, x)
if (CFP10 && !CFP11 && N1.getOpcode() == ISD::FADD &&
@@ -5914,6 +5901,26 @@
}
}
+ if (N0.getOpcode() == ISD::FADD) {
+ ConstantFPSDNode *CFP = dyn_cast<ConstantFPSDNode>(N0.getOperand(0));
+ // (fadd (fadd x, x), x) -> (fmul 3.0, x)
+ if (!CFP && N0.getOperand(0) == N0.getOperand(1) &&
+ (N0.getOperand(0) == N1)) {
+ return DAG.getNode(ISD::FMUL, N->getDebugLoc(), VT,
+ N1, DAG.getConstantFP(3.0, VT));
+ }
+ }
+
+ if (N1.getOpcode() == ISD::FADD) {
+ ConstantFPSDNode *CFP10 = dyn_cast<ConstantFPSDNode>(N1.getOperand(0));
+ // (fadd x, (fadd x, x)) -> (fmul 3.0, x)
+ if (!CFP10 && N1.getOperand(0) == N1.getOperand(1) &&
+ N1.getOperand(0) == N0) {
+ return DAG.getNode(ISD::FMUL, N->getDebugLoc(), VT,
+ N0, DAG.getConstantFP(3.0, VT));
+ }
+ }
+
// (fadd (fadd x, x), (fadd x, x)) -> (fmul 4.0, x)
if (N0.getOpcode() == ISD::FADD && N1.getOpcode() == ISD::FADD &&
N0.getOperand(0) == N0.getOperand(1) &&
More information about the llvm-commits
mailing list