[AArch64] fold 'isPositive' vector integer operations (PR26819)

[spatel]

Summary

This is one of the cases shown in:
https://llvm.org/bugs/show_bug.cgi?id=26819

Shift and negate is what InstCombine prefers to produce (and I tried to make it do more of that in http://reviews.llvm.org/rL262424 ), so we should recognize that pattern as something that might come from autovectorization even if it's unlikely to be produced from C NEON intrinsics.

The patch is largely a copy/paste of the x86 equivalent:
http://reviews.llvm.org/rL262036

...so it might be worth noting as something that could be refactored into DAGCombiner so all backends can share it.

[t.p.northover]

Thanks for working on this Sanjay, especially as it's not really your home target. Just one simplification to suggest:

lib/Target/AArch64/AArch64ISelLowering.cpp

7447 ↗

(On Diff #49682)

This looks like a vestige of x86's different levels of vector support. On AArch64 all vector types are legal or none are.

So I'd suggest making this *just* the NEON check (or even an assertion), and moving the foldVectorXorShiftIntoCmp call below the isBeforeLegalizeOps check.

[t.p.northover]

lib/Target/AArch64/AArch64ISelLowering.cpp

7447 ↗

(On Diff #49682)

Oh, actually you probably do need an isVectorType check to avoid doing nasty things to scalars.

[spatel]

Patch updated based on Tim's suggestion: wait until after legalization, and it makes the code simpler!

[t.p.northover]

Looks good. Thanks again!

Tim.

llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp

if (BitSize == 32)
     Val &= (1LL << 32) - 1;
 
   unsigned LZ = countLeadingZeros((uint64_t)Val);
   unsigned Shift = (63 - LZ) / 16;
   // MOVZ is free so return true for one or fewer MOVK.
   return Shift < 3;
 }
 
+/// Turn vector tests of the signbit in the form of:
+///   xor (sra X, elt_size(X)-1), -1
+/// into:
+///   cmge X, X, #0
+static SDValue foldVectorXorShiftIntoCmp(SDNode *N, SelectionDAG &DAG,
+                                         const AArch64Subtarget *Subtarget) {
+  EVT VT = N->getValueType(0);
+  if (!Subtarget->hasNEON() || !VT.isVector())
+    return SDValue();
+
+  // There must be a shift right algebraic before the xor, and the xor must be a
+  // 'not' operation.
+  SDValue Shift = N->getOperand(0);
+  SDValue Ones = N->getOperand(1);
+  if (Shift.getOpcode() != AArch64ISD::VASHR || !Shift.hasOneUse() ||
+      !ISD::isBuildVectorAllOnes(Ones.getNode()))
+    return SDValue();
+
+  // The shift should be smearing the sign bit across each vector element.
+  auto *ShiftAmt = dyn_cast<ConstantSDNode>(Shift.getOperand(1));
+  EVT ShiftEltTy = Shift.getValueType().getVectorElementType();
+  if (!ShiftAmt || ShiftAmt->getZExtValue() != ShiftEltTy.getSizeInBits() - 1)
+    return SDValue();
+
+  return DAG.getNode(AArch64ISD::CMGEz, SDLoc(N), VT, Shift.getOperand(0));
+}
+
 // Generate SUBS and CSEL for integer abs.
 static SDValue performIntegerAbsCombine(SDNode *N, SelectionDAG &DAG) {
   EVT VT = N->getValueType(0);
 
   SDValue N0 = N->getOperand(0);
   SDValue N1 = N->getOperand(1);
   SDLoc DL(N);
 
if (ConstantSDNode *Y1C = dyn_cast<ConstantSDNode>(N1.getOperand(1)))
                         N0.getOperand(0), DAG.getConstant(0, DL, VT));
         return DAG.getNode(AArch64ISD::CSEL, DL, VT, N0.getOperand(0), Neg,
                            DAG.getConstant(AArch64CC::PL, DL, MVT::i32),
                            SDValue(Cmp.getNode(), 1));
       }
   return SDValue();
 }
 
-// performXorCombine - Attempts to handle integer ABS.
 static SDValue performXorCombine(SDNode *N, SelectionDAG &DAG,
                                  TargetLowering::DAGCombinerInfo &DCI,
                                  const AArch64Subtarget *Subtarget) {
   if (DCI.isBeforeLegalizeOps())
     return SDValue();
 
+  if (SDValue Cmp = foldVectorXorShiftIntoCmp(N, DAG, Subtarget))
+    return Cmp;
+
   return performIntegerAbsCombine(N, DAG);
 }
 
 SDValue
 AArch64TargetLowering::BuildSDIVPow2(SDNode *N, const APInt &Divisor,
                                      SelectionDAG &DAG,
                                      std::vector<SDNode *> *Created) const {
   // fold (sdiv X, pow2)

llvm/trunk/test/CodeGen/AArch64/neon-compare-instructions.ll

 define <2 x i64> @cmgez2xi64(<2 x i64> %A) {
 ; CHECK-LABEL: cmgez2xi64:
 ; CHECK: cmge {{v[0-9]+}}.2d, {{v[0-9]+}}.2d, #{{0x0|0}}
   %tmp3 = icmp sge <2 x i64> %A, zeroinitializer;
    %tmp4 = sext <2 x i1> %tmp3 to <2 x i64>
   ret <2 x i64> %tmp4
 }
 
-; FIXME: The following 7 tests could be optimized to cmgez to save an instruction.
 
 define <8 x i8> @cmgez8xi8_alt(<8 x i8> %A) {
 ; CHECK-LABEL: cmgez8xi8_alt:
-; CHECK:       sshr {{v[0-9]+}}.8b, {{v[0-9]+}}.8b, #7
-; CHECK-NEXT:  {{mvn|not}} {{v[0-9]+}}.8b, {{v[0-9]+}}.8b
+; CHECK: cmge {{v[0-9]+}}.8b, {{v[0-9]+}}.8b, #{{0x0|0}}
   %sign = ashr <8 x i8> %A, <i8 7, i8 7, i8 7, i8 7, i8 7, i8 7, i8 7, i8 7>
   %not = xor <8 x i8> %sign, <i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1>
   ret <8 x i8> %not
 }
 
 define <16 x i8> @cmgez16xi8_alt(<16 x i8> %A) {
 ; CHECK-LABEL: cmgez16xi8_alt:
-; CHECK:       sshr {{v[0-9]+}}.16b, {{v[0-9]+}}.16b, #7
-; CHECK-NEXT:  {{mvn|not}} {{v[0-9]+}}.16b, {{v[0-9]+}}.16b
+; CHECK: cmge {{v[0-9]+}}.16b, {{v[0-9]+}}.16b, #{{0x0|0}}
   %sign = ashr <16 x i8> %A, <i8 7, i8 7, i8 7, i8 7, i8 7, i8 7, i8 7, i8 7, i8 7, i8 7, i8 7, i8 7, i8 7, i8 7, i8 7, i8 7>
   %not = xor <16 x i8> %sign, <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>
   ret <16 x i8> %not
 }
 
 define <4 x i16> @cmgez4xi16_alt(<4 x i16> %A) {
 ; CHECK-LABEL: cmgez4xi16_alt:
-; CHECK:       sshr {{v[0-9]+}}.4h, {{v[0-9]+}}.4h, #15
-; CHECK-NEXT:  {{mvn|not}} {{v[0-9]+}}.8b, {{v[0-9]+}}.8b
+; CHECK: cmge {{v[0-9]+}}.4h, {{v[0-9]+}}.4h, #{{0x0|0}}
   %sign = ashr <4 x i16> %A, <i16 15, i16 15, i16 15, i16 15>
   %not = xor <4 x i16> %sign, <i16 -1, i16 -1, i16 -1, i16 -1>
   ret <4 x i16> %not
 }
 
 define <8 x i16> @cmgez8xi16_alt(<8 x i16> %A) {
 ; CHECK-LABEL: cmgez8xi16_alt:
-; CHECK:       sshr {{v[0-9]+}}.8h, {{v[0-9]+}}.8h, #15
-; CHECK-NEXT:  {{mvn|not}} {{v[0-9]+}}.16b, {{v[0-9]+}}.16b
+; CHECK: cmge {{v[0-9]+}}.8h, {{v[0-9]+}}.8h, #{{0x0|0}}
   %sign = ashr <8 x i16> %A, <i16 15, i16 15, i16 15, i16 15, i16 15, i16 15, i16 15, i16 15>
   %not = xor <8 x i16> %sign, <i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1>
   ret <8 x i16> %not
 }
 
 define <2 x i32> @cmgez2xi32_alt(<2 x i32> %A) {
 ; CHECK-LABEL: cmgez2xi32_alt:
-; CHECK:       sshr {{v[0-9]+}}.2s, {{v[0-9]+}}.2s, #31
-; CHECK-NEXT:  {{mvn|not}} {{v[0-9]+}}.8b, {{v[0-9]+}}.8b
+; CHECK: cmge {{v[0-9]+}}.2s, {{v[0-9]+}}.2s, #{{0x0|0}}
   %sign = ashr <2 x i32> %A, <i32 31, i32 31>
   %not = xor <2 x i32> %sign, <i32 -1, i32 -1>
   ret <2 x i32> %not
 }
 
 define <4 x i32> @cmgez4xi32_alt(<4 x i32> %A) {
 ; CHECK-LABEL: cmgez4xi32_alt:
-; CHECK:       sshr {{v[0-9]+}}.4s, {{v[0-9]+}}.4s, #31
-; CHECK-NEXT:  {{mvn|not}} {{v[0-9]+}}.16b, {{v[0-9]+}}.16b
+; CHECK: cmge {{v[0-9]+}}.4s, {{v[0-9]+}}.4s, #{{0x0|0}}
   %sign = ashr <4 x i32> %A, <i32 31, i32 31, i32 31, i32 31>
   %not = xor <4 x i32> %sign, <i32 -1, i32 -1, i32 -1, i32 -1>
   ret <4 x i32> %not
 }
 
 define <2 x i64> @cmgez2xi64_alt(<2 x i64> %A) {
 ; CHECK-LABEL: cmgez2xi64_alt:
-; CHECK:       sshr {{v[0-9]+}}.2d, {{v[0-9]+}}.2d, #63
-; CHECK-NEXT:  {{mvn|not}} {{v[0-9]+}}.16b, {{v[0-9]+}}.16b
+; CHECK: cmge {{v[0-9]+}}.2d, {{v[0-9]+}}.2d, #{{0x0|0}}
   %sign = ashr <2 x i64> %A, <i64 63, i64 63>
   %not = xor <2 x i64> %sign, <i64 -1, i64 -1>
   ret <2 x i64> %not
 }
 
 
 define <8 x i8> @cmgtz8xi8(<8 x i8> %A) {
 ; CHECK-LABEL: cmgtz8xi8: