[AArch64] Avoid materializing constant values when generating csel instructions.

[mcrosier]

Summary

Please take a look.

Chad

[efriedma]

test/CodeGen/AArch64/arm64-csel.ll

241

Currently, trunk generates the following for foo20:

cmp     w0, #1
orr     w8, wzr, #0x1
cneg    w0, w8, ne
ret

This is "w0 == 1 ? 1 : -1"

With your patch, we generate... this?

cmp     w0, #1
cneg    w0, w0, ne
ret

Unless I'm missing something, this is "w0 == 1 ? w0 : -w0", which is not the same thing.

[mcrosier]

test/CodeGen/AArch64/arm64-csel.ll

241

Ah, yes.. For example, if w0 holds 5 the cneg would put -5 in w0, rather than -1.

[mcrosier]

I've uploaded a new patch, which was my original solution to solving PR28965. My first patch tried to be more general, but obviously made an incorrect assumption.

[efriedma]

lib/Target/AArch64/AArch64ISelLowering.cpp

4001

There are really two transformations going on here...

1. You can avoid materializing -1 using CSINV on the zero register. (Similarly, you can avoid materializing 1 with CSINC on the zero register.)
2. You can transform "a == C ? C : x" to "a == C ? a : x" to avoid materializing C.

We want to perform each of these transformations whether or not the other is possible.

[mcrosier]

lib/Target/AArch64/AArch64ISelLowering.cpp

4001

I made the same realization last night.

I've been working on a new patch this morning. This general transform (#2) is applicable to CSEL, CSNEG, CSINV, etc. Transform #1 is specific to CSINV and is independent of tranform #2 as you've already pointed out.

I'll post a revised patch shortly.

[mcrosier]

-Address Eli's comment.

[efriedma]

Possibly interesting testcase: "x == 7 ? 7 : -1". Should generate cmp+csinv. Also, the related "x == 7 ? 7 : 1" should generate cmp+csinc.

lib/Target/AArch64/AArch64ISelLowering.cpp

4014

Maybe you could just make this fall through to the general case, rather than explicitly selecting CSINV?

4017

Maybe it makes sense to write something more like if ((TrueVal == SignExtend64(FalseVal + 1, ValueBits)) || (SignExtend64(TrueVal + 1, ValueBits) == FalseVal)) {. This also applies to your later code.

[mcrosier]

I found very few cases where the csinc/csinv optimizations actually hit, so I'm limiting this patch to just csel to simplify the logic/review.

[mcrosier]

lib/Target/AArch64/AArch64ISelLowering.cpp

4014

Deferring this work for another day.

4017

Again, deferring this for now.

[gberry]

lib/Target/AArch64/AArch64ISelLowering.cpp

4051

typo "know" -> "known"

[mcrosier]

Address Geoff's comment.

[efriedma]

LGTM.

lib/Target/AArch64/AArch64ISelLowering.cpp

if (CTVal && CFVal && CTVal->isAllOnesValue() && CFVal->isNullValue()) {
       if (isNullConstant(TVal.getOperand(0))) {
         std::swap(TVal, FVal);
         std::swap(CTVal, CFVal);
         CC = ISD::getSetCCInverse(CC, true);
       }
     } else if (CTVal && CFVal) {
       const int64_t TrueVal = CTVal->getSExtValue();
       const int64_t FalseVal = CFVal->getSExtValue();
+      ConstantSDNode *RHSVal = dyn_cast<ConstantSDNode>(RHS);
       bool Swap = false;
+      bool KeepFVal = false;
 
       // If both TVal and FVal are constants, see if FVal is the
       // inverse/negation/increment of TVal and generate a CSINV/CSNEG/CSINC
       // instead of a CSEL in that case.
       if (TrueVal == ~FalseVal) {
         Opcode = AArch64ISD::CSINV;
+      } else if (CFVal->isAllOnesValue() && RHSVal == CTVal &&
+                 changeIntCCToAArch64CC(CC) == AArch64CC::EQ) {
+        // If FVal is -1, we can use a CSINV to invert wzr/xzr to avoid
+        // materializing the -1.  We can also avoid materializing TVal, if TVal
+        // matches RHS and we're doing an equality comparison as we know LHS
+        // will contain TVal if the equality is true.
+        Opcode = AArch64ISD::CSINV;
+        TVal = LHS;
+        FVal = DAG.getConstant(0, dl, FVal.getValueType());
+        KeepFVal = true;
       } else if (TrueVal == -FalseVal) {
         Opcode = AArch64ISD::CSNEG;
       } else if (TVal.getValueType() == MVT::i32) {
         // If our operands are only 32-bit wide, make sure we use 32-bit
         // arithmetic for the check whether we can use CSINC. This ensures that
         // the addition in the check will wrap around properly in case there is
         // an overflow (which would not be the case if we do the check with
         // 64-bit arithmetic).
         const uint32_t TrueVal32 = CTVal->getZExtValue();
         const uint32_t FalseVal32 = CFVal->getZExtValue();
 
         if ((TrueVal32 == FalseVal32 + 1) || (TrueVal32 + 1 == FalseVal32)) {
           Opcode = AArch64ISD::CSINC;
 
           if (TrueVal32 > FalseVal32) {
             Swap = true;
if (CTVal && CFVal && CTVal->isAllOnesValue() && CFVal->isNullValue()) {
 
       // Swap TVal and FVal if necessary.
       if (Swap) {
         std::swap(TVal, FVal);
         std::swap(CTVal, CFVal);
         CC = ISD::getSetCCInverse(CC, true);
       }
 
-      if (Opcode != AArch64ISD::CSEL) {
+      if (Opcode != AArch64ISD::CSEL && !KeepFVal) {
         // Drop FVal since we can get its value by simply inverting/negating
         // TVal.
         FVal = TVal;
       }
     }
 
     SDValue CCVal;
     SDValue Cmp = getAArch64Cmp(LHS, RHS, CC, CCVal, DAG, dl);
 
     EVT VT = TVal.getValueType();
     return DAG.getNode(Opcode, dl, VT, TVal, FVal, CCVal, Cmp);
   }
 
   // Now we know we're dealing with FP values.
   assert(LHS.getValueType() == MVT::f32 || LHS.getValueType() == MVT::f64);

test/CodeGen/AArch64/arm64-csel.ll

 ; CHECK-LABEL: foo19:
 ; CHECK: cinc x0, x2
 ; CHECK-NOT: add
   %cmp = icmp ult i64 %a, %b
   %inc = zext i1 %cmp to i64
   %inc.c = add i64 %inc, %c
   ret i64 %inc.c
 }
+
+; PR28965
+define i32 @foo20(i32 %x) {
+entry:
+; CHECK-LABEL: foo20:
+; CHECK: cmp w[[REG:[0-9]+]], #1
+; CHECK-NOT: orr w{{[0-9]+}}, wzr, #0x1
+; CHECK: csinv w0, w[[REG]], wzr, eq
+  %cmp = icmp eq i32 %x, 1
+  %res = select i1 %cmp, i32 1, i32 -1
+  ret i32 %res
+}
+
+define i64 @foo21(i64 %x) {
+entry:
+; CHECK-LABEL: foo21:
+; CHECK: cmp x[[REG:[0-9]+]], #1
+; CHECK-NOT: orr w{{[0-9]+}}, wzr, #0x1
+; CHECK: csinv x0, x[[REG]], xzr, eq
+  %cmp = icmp eq i64 %x, 1
+  %res = select i1 %cmp, i64 1, i64 -1
+  ret i64 %res
+}
+
+define i64 @foo22(i64 %x) {
+entry:
+; CHECK-LABEL: foo22:
+; CHECK: cmp x[[REG:[0-9]+]], #3
+; CHECK-NOT: orr w{{[0-9]+}}, wzr, #0x3
+; CHECK: csinv x0, x[[REG]], xzr, eq
+  %cmp = icmp eq i64 %x, 3
+  %res = select i1 %cmp, i64 3, i64 -1
+  ret i64 %res
+}