[AArch64 NEON] Invalid constant is used in vselect condition.

[kevin.qin]

Summary

Hi,

In AArch64 backend, If the conditions of vselect are vector constant, then the value of "true" is presented as (i8 1) instead of (i8 255) which we expect.
The reason is, vselect only accept vxi1 as condition operand while BSL use every bit of condition to make select. So legalizer will insert a sign_extend_inreg to promote vxi1 to vxi8. Next DAGCombiner will use visitSIGN_EXTEND_INREG() to combine it with a build_vector. Because in AArch64, i8 is illegal, constants with i1 are directly promoted to i32, so for i32, left shift 7 and then arithmetic right shift 7 can't correctly sign_extend i1 to i8.

[andreadb]

Hi Kevin,

your patch looks good to me.
My original code worked under the wrong assumption that the vector element type and the type of each ConstantSDNode in the build_vector were the same.
However, when promoting the integer operand of a legally typed build_vector, the operand type and the vector element type do not need to be the same (See method 'DAGTypeLegalizer::PromoteIntOp_BUILD_VECTOR' in LegalizeIntegerTypes.cpp).

in AArch64 backend, the following dag sequence:

C0: i1 = Constant<0>
C1: i1 = Constant<-1>
V: v8i1 = BUILD_VECTOR C1, C1, C0, C0, C0, C0, C0, C0

is type-legalized into:

NewC0: i32 = Constant<0>
NewC1: i32 = Constant<1>
V: v8i8 = BUILD_VECTOR NewC1, NewC1, NewC0, NewC0, NewC0, NewC0, NewC0, NewC0

Forcing a getZeroExtend to VTBits to ensure that the new constant is correctly generated looks good to me!

Thanks,
Andrea Di Biagio

[Eugene.Zelenko]

Committed in rL198582.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

       }
 
       ConstantSDNode *CurrentND = cast<ConstantSDNode>(Op);
-      const APInt &C = CurrentND->getAPIntValue();
-      Elts.push_back(DAG.getConstant(C.shl(ShAmt).ashr(ShAmt),
+      const APInt &C = APInt(VTBits, CurrentND->getAPIntValue().getZExtValue());
+      Elts.push_back(DAG.getConstant(C.shl(ShAmt).ashr(ShAmt).getZExtValue(),
                                      Op.getValueType()));
     }
 

test/CodeGen/AArch64/neon-bitwise-instructions.ll

   ret <4 x i32> %4
 }
 
+define <8 x i8> @vselect_v8i8(<8 x i8> %a) {
+;CHECK:  movi  {{d[0-9]+}}, #0xffff
+;CHECK-NEXT:  bsl {{v[0-9]+}}.8b, {{v[0-9]+}}.8b, {{v[0-9]+}}.8b
+  %b = select <8 x i1> <i1 true, i1 true, i1 false, i1 false, i1 false, i1 false, i1 false, i1 false>, <8 x i8> %a, <8 x i8> <i8 undef, i8 undef, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0>
+  ret <8 x i8> %b
+}
+
+define <4 x i16> @vselect_v4i16(<4 x i16> %a) {
+;CHECK:  movi  {{d[0-9]+}}, #0xffff
+;CHECK-NEXT:  bsl {{v[0-9]+}}.8b, {{v[0-9]+}}.8b, {{v[0-9]+}}.8b
+  %b = select <4 x i1> <i1 true, i1 false, i1 false, i1 false>, <4 x i16> %a, <4 x i16> <i16 undef, i16 0, i16 0, i16 0>
+  ret <4 x i16> %b
+}
+
+define <8 x i8> @vselect_cmp_ne(<8 x i8> %a, <8 x i8> %b, <8 x i8> %c) {
+;CHECK:  cmeq {{v[0-9]+}}.8b, {{v[0-9]+}}.8b, {{v[0-9]+}}.8b
+;CHECK-NEXT:  not {{v[0-9]+}}.8b, {{v[0-9]+}}.8b
+;CHECK-NEXT:  bsl {{v[0-9]+}}.8b, {{v[0-9]+}}.8b, {{v[0-9]+}}.8b
+  %cmp = icmp ne <8 x i8> %a, %b
+  %d = select <8 x i1> %cmp, <8 x i8> %b, <8 x i8> %c
+  ret <8 x i8> %d
+}
+
+define <8 x i8> @vselect_cmp_eq(<8 x i8> %a, <8 x i8> %b, <8 x i8> %c) {
+;CHECK:  cmeq {{v[0-9]+}}.8b, {{v[0-9]+}}.8b, {{v[0-9]+}}.8b
+;CHECK-NEXT:  bsl {{v[0-9]+}}.8b, {{v[0-9]+}}.8b, {{v[0-9]+}}.8b
+  %cmp = icmp eq <8 x i8> %a, %b
+  %d = select <8 x i1> %cmp, <8 x i8> %b, <8 x i8> %c
+  ret <8 x i8> %d
+}
+
+define <8 x i8> @vselect_cmpz_ne(<8 x i8> %a, <8 x i8> %b, <8 x i8> %c) {
+;CHECK:  cmeq {{v[0-9]+}}.8b, {{v[0-9]+}}.8b, #0
+;CHECK-NEXT:  not {{v[0-9]+}}.8b, {{v[0-9]+}}.8b
+;CHECK-NEXT:  bsl {{v[0-9]+}}.8b, {{v[0-9]+}}.8b, {{v[0-9]+}}.8b
+  %cmp = icmp ne <8 x i8> %a, zeroinitializer
+  %d = select <8 x i1> %cmp, <8 x i8> %b, <8 x i8> %c
+  ret <8 x i8> %d
+}
+
+define <8 x i8> @vselect_cmpz_eq(<8 x i8> %a, <8 x i8> %b, <8 x i8> %c) {
+;CHECK:  cmeq {{v[0-9]+}}.8b, {{v[0-9]+}}.8b, #0
+;CHECK-NEXT:  bsl {{v[0-9]+}}.8b, {{v[0-9]+}}.8b, {{v[0-9]+}}.8b
+  %cmp = icmp eq <8 x i8> %a, zeroinitializer
+  %d = select <8 x i1> %cmp, <8 x i8> %b, <8 x i8> %c
+  ret <8 x i8> %d
+}
+
+define <8 x i8> @vselect_tst(<8 x i8> %a, <8 x i8> %b, <8 x i8> %c) {
+;CHECK:  cmtst {{v[0-9]+}}.8b, {{v[0-9]+}}.8b, {{v[0-9]+}}.8b
+;CHECK-NEXT:  bsl {{v[0-9]+}}.8b, {{v[0-9]+}}.8b, {{v[0-9]+}}.8b
+  %tmp3 = and <8 x i8> %a, %b
+  %tmp4 = icmp ne <8 x i8> %tmp3, zeroinitializer
+  %d = select <8 x i1> %tmp4, <8 x i8> %b, <8 x i8> %c
+  ret <8 x i8> %d
+}
+
 define <2 x i64> @bsl2xi64(<2 x i64> %v1, <2 x i64> %v2, <2 x i64> %v3) {
 ;CHECK:  bsl {{v[0-9]+}}.16b, {{v[0-9]+}}.16b, {{v[0-9]+}}.16b
   %1 = and <2 x i64> %v1, %v2