[PATCH][AArch64] Fix an assertion failure in function 'PerformORCombine' caused by an invalid comparison between APInt values with different BitWidth.

Andrea Di Biagio andrea.dibiagio at gmail.com
Wed Jan 8 05:27:26 PST 2014


Hi,

While looking at the expansion of vselect dag nodes on non-x86
targets, I found a bug in a target specific combine transformation
which affects the AArch64 target with neon support.

The AArch64 backend knows how to optimize a 'or' dag node according to rule:
  (or (and B, A), (and C, D)) => (VBSL A, B, C)
where D is equal to ~A.

Function PerformORCombine (In AArch64ISelLowering.cpp) firstly checks
that operands A and D are actually build_vector dag nodes; then it
calls method method 'isConstantSplat' on both A and D to get their
associated splat values and eventually compare them to see if D is
equal to ~A.

The problem is that splat values (APInt instances) returned by method
'isConstantSplat' may differ in BitWidth. APInt only knows how to
compare numbers with the same BitWidth and asserts in all other cases.

Before this fix, DAGCombiner failed with an assertion when trying to
'PerformORCombine' on the test cases from
'test/CodeGen/AArch64/neon-or-combine.ll' (see the patch).

This patch fixes the problem introducing an explicit check to verify
that BitWidths are the same for two splat values before comparing
them.

Please let me know if ok to submit.

Thanks,
Andrea Di Biagio
SN Systems - Sony Computer Entertainment Group
-------------- next part --------------
Index: test/CodeGen/AArch64/neon-or-combine.ll
===================================================================
--- test/CodeGen/AArch64/neon-or-combine.ll	(revision 0)
+++ test/CodeGen/AArch64/neon-or-combine.ll	(revision 0)
@@ -0,0 +1,29 @@
+; RUN: llc < %s -mtriple=aarch64-none-linux-gnu -mattr=+neon | FileCheck %s
+
+; Check that the DAGCombiner does not crash with an assertion failure
+; when performing a target specific combine to simplify a 'or' dag node
+; according to the following rule:
+;   (or (and B, A), (and C, ~A)) => (VBSL A, B, C)
+; The assertion failure was caused by an invalid comparison between APInt
+; values with different 'BitWidth'.
+
+define <8 x i8> @test1(<8 x i8> %a, <8 x i8> %b)  {
+  %tmp1 = and <8 x i8> %a, < i8 -1, i8 -1, i8 0, i8 0, i8 -1, i8 -1, i8 0, i8 0 >
+  %tmp2 = and <8 x i8> %b, < i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0 >
+  %tmp3 = or <8 x i8> %tmp1, %tmp2
+  ret <8 x i8> %tmp3
+}
+
+; CHECK-LABEL: test1
+; CHECK: ret
+
+define <16 x i8> @test2(<16 x i8> %a, <16 x i8> %b) {
+  %tmp1 = and <16 x i8> %a, < i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1 >
+  %tmp2 = and <16 x i8> %b, < i8 -1, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0 >
+  %tmp3 = or <16 x i8> %tmp1, %tmp2
+  ret <16 x i8> %tmp3
+}
+
+; CHECK-LABEL: test2
+; CHECK: ret
+
Index: lib/Target/AArch64/AArch64ISelLowering.cpp
===================================================================
--- lib/Target/AArch64/AArch64ISelLowering.cpp	(revision 198764)
+++ lib/Target/AArch64/AArch64ISelLowering.cpp	(working copy)
@@ -3480,8 +3480,9 @@
       BuildVectorSDNode *BVN1 = dyn_cast<BuildVectorSDNode>(N1->getOperand(1));
       APInt SplatBits1;
       if (BVN1 && BVN1->isConstantSplat(SplatBits1, SplatUndef, SplatBitSize,
-                                        HasAnyUndefs) &&
-          !HasAnyUndefs && SplatBits0 == ~SplatBits1) {
+                                        HasAnyUndefs) && !HasAnyUndefs &&
+          SplatBits0.getBitWidth() == SplatBits1.getBitWidth() &&
+          SplatBits0 == ~SplatBits1) {
 
         return DAG.getNode(ISD::VSELECT, DL, VT, N0->getOperand(1),
                            N0->getOperand(0), N1->getOperand(0));


More information about the llvm-commits mailing list