[DAGCombiner] Fix a regression in method SimplifyVBinOp accidentally introduced by r199135.

Andrea Di Biagio andrea.dibiagio at gmail.com
Wed Jan 15 04:50:15 PST 2014


Hi,

this patch fixes a bug introduced by revision 199135 in method
DAGCombiner::SimplifyVBinOp'.

Revision 199135 tried to simplify part of the logic in method
DAGCombiner::SimplifyVBinOp' introducing calls to method
'BuildVectorSDNode::isConstant().

However, that revision wrongly changed the check performed by method
DAGCombiner::SimplifyVBinOp to identify dag nodes that can be folded.

Before revision 199135, that method only tried to simplify vector
binary operations if both operands were build_vector of
Constant/ConstantFP/Undef only.

After revision 199135, method 'SimplifyVBinOp' behaves differently; it
attempts to simplify vector binary operations even if only one of the
two operands is a constant.

The regression was originally reported by Patrik Hägglund H.
I reduced the original test case provided by Patrik and adapted it to
the regression suite (see file
test/CodeGen/X86/vbinop-simplify-bug.ll).

Please let me know if ok to submit.

Thanks,
Andrea Di Biagio
SN Systems - Sony Computer Entertainment Group.
-------------- next part --------------
Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/DAGCombiner.cpp	(revision 199314)
+++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp	(working copy)
@@ -10391,8 +10391,8 @@
   if (LHS.getOpcode() == ISD::BUILD_VECTOR &&
       RHS.getOpcode() == ISD::BUILD_VECTOR) {
     // Check if both vectors are constants. If not bail out.
-    if (!cast<BuildVectorSDNode>(LHS)->isConstant() &&
-        !cast<BuildVectorSDNode>(RHS)->isConstant())
+    if (!(cast<BuildVectorSDNode>(LHS)->isConstant() &&
+          cast<BuildVectorSDNode>(RHS)->isConstant()))
       return SDValue();
 
     SmallVector<SDValue, 8> Ops;
Index: test/CodeGen/X86/vbinop-simplify-bug.ll
===================================================================
--- test/CodeGen/X86/vbinop-simplify-bug.ll	(revision 0)
+++ test/CodeGen/X86/vbinop-simplify-bug.ll	(revision 0)
@@ -0,0 +1,26 @@
+; RUN: llc < %s -mtriple="x86_64-unknown-linux-gnu" -mattr=sse2 -mcpu=corei7 | FileCheck %s
+
+; Revision 199135 introduced a wrong check in method
+; DAGCombiner::SimplifyVBinOp in an attempt to refactor some code
+; using the new method 'BuildVectorSDNode::isConstant' when possible.
+; 
+; However the modified code in method SimplifyVBinOp now wrongly
+; checks that the operands of a vector bin-op are both constants.
+;
+; With that wrong change, this test started failing because of a
+; 'fatal error in the backend':
+;   Cannot select: 0x2e329d0: v4i32 = BUILD_VECTOR 0x2e2ea00, 0x2e2ea00, 0x2e2ea00, 0x2e2ea00
+;       0x2e2ea00: i32 = Constant<1> [ID=4]
+;       0x2e2ea00: i32 = Constant<1> [ID=4]
+;       0x2e2ea00: i32 = Constant<1> [ID=4]
+;       0x2e2ea00: i32 = Constant<1> [ID=4]
+
+define <8 x i32> @reduced_test_case() {
+  %Shuff = shufflevector <8 x i32> zeroinitializer, <8 x i32> zeroinitializer, <8 x i32> <i32 1, i32 3, i32 undef, i32 7, i32 9, i32 11, i32 13, i32 15>
+  %B23 = sub <8 x i32> %Shuff, <i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1>
+  ret <8 x i32> %B23
+}
+
+; CHECK-LABEL: reduced_test_case
+; CHECK: ret
+


More information about the llvm-commits mailing list