[AArch64] Combine ISD::AND into AArch64ISD::ANDS

[dmgreen]

Summary

If we already have a AArch64ISD::ANDS node with identical operands, we can merge any ISD::AND into it, reducing the instruction count by calculating the value and the flags in a single operation. This code is taken from the X86 backend, and could also handle AArch64ISD::ADDS and AArch64ISD::SUBS, but I couldn't find any test cases where it came up.

[david-arm]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

14372

Is there any need to pass in GenericOpc as it seems to always be ISD::AND?

14382

Do you have to do this given we're not merging any results together? For example, does it work if you just do

return DCI.DAG.getNode(GenericOpc, DL, VT, LHS, RHS);

?

[dmgreen]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

14372

Did you see the Note 2 lines up :)

This code can also be used for AArch64::ADDS and AArch64::SUBS, I just couldn't find a testcase where it modified things yet. On X86 they lower certain x86 intrinsics straight to the nodes with make them easier to write tests for.

14382

Hmm. A node that produces 2 results needs to be replaced by a node the produces 2 results.

[david-arm]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

14372

I did indeed. :) And despite that I still wondered if it was worth adding the extra argument at this point given there isn't any evidence that it's needed yet.

[dmgreen]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

14372

Hmm. I think it makes the function less generic, and it would be good to re-use it for ADDS/SUBS. I feel like that's just worse. But then I did rename the functions already (I didn't like the old name it had), and never found a good test case for the ADDS/SUBS which was a shame.

[dmgreen]

Remove the GenericOpc argument.

[samtebbs]

Nice one. LGTM if david has no other comments

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

17355

ANDS -> ADDS

[david-arm]

LGTM as well! Sorry I'd forgotten about this and should have responded much earlier!

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

static SDValue performANDCombine(SDNode *N,
   }
 
   return SDValue();
 }
 
 static bool hasPairwiseAdd(unsigned Opcode, EVT VT, bool FullFP16) {
   switch (Opcode) {
   case ISD::STRICT_FADD:
   case ISD::FADD:
     return (FullFP16 && VT == MVT::f16) || VT == MVT::f32 || VT == MVT::f64;
   case ISD::ADD:
     return VT == MVT::i64;
   default:
     return false;
   }
 }
 
 static SDValue getPTest(SelectionDAG &DAG, EVT VT, SDValue Pg, SDValue Op,
                         AArch64CC::CondCode Cond);
 
 // Materialize : i1 = extract_vector_elt t37, Constant:i64<0>
 // ... into: "ptrue p, all" + PTEST
 static SDValue
 performFirstTrueTestVectorCombine(SDNode *N,
                                   TargetLowering::DAGCombinerInfo &DCI,
                                   const AArch64Subtarget *Subtarget) {
   assert(N->getOpcode() == ISD::EXTRACT_VECTOR_ELT);
SDValue CSEL =
                     LHS.getOperand(1), DAG.getConstant(NewCond, DL, MVT::i32),
                     LHS.getOperand(3));
     return DAG.getZExtOrTrunc(CSEL, DL, N->getValueType(0));
   }
 
   return SDValue();
 }
 
+// Combines for S forms of generic opcodes (AArch64ISD::ANDS into ISD::AND for
+// example). NOTE: This could be used for ADDS and SUBS too, if we can find test
+// cases.
+static SDValue performANDSCombine(SDNode *N,
+                                  TargetLowering::DAGCombinerInfo &DCI) {
+  SDLoc DL(N);
+  SDValue LHS = N->getOperand(0);
+  SDValue RHS = N->getOperand(1);
+  EVT VT = N->getValueType(0);
+
+  // If the flag result isn't used, convert back to a generic opcode.
+  if (!N->hasAnyUseOfValue(1)) {
+    SDValue Res = DCI.DAG.getNode(ISD::AND, DL, VT, LHS, RHS);
+    return DCI.DAG.getMergeValues({Res, DCI.DAG.getConstant(0, DL, MVT::i32)},
+                                  DL);
+  }
+
+  // Combine identical generic nodes into this node, re-using the result.
+  if (SDNode *GenericAddSub =
+          DCI.DAG.getNodeIfExists(ISD::AND, DCI.DAG.getVTList(VT), {LHS, RHS}))
+    DCI.CombineTo(GenericAddSub, SDValue(N, 0));
+
+  return SDValue();
+}
+
 static SDValue performSetCCPunpkCombine(SDNode *N, SelectionDAG &DAG) {
   // setcc_merge_zero pred
   //   (sign_extend (extract_subvector (setcc_merge_zero ... pred ...))), 0, ne
   //   => extract_subvector (inner setcc_merge_zero)
   SDValue Pred = N->getOperand(0);
   SDValue LHS = N->getOperand(1);
   SDValue RHS = N->getOperand(2);
   ISD::CondCode Cond = cast<CondCodeSDNode>(N->getOperand(3))->get();
case ISD::FP_EXTEND:
     return performFPExtendCombine(N, DAG, DCI, Subtarget);
   case AArch64ISD::BRCOND:
     return performBRCONDCombine(N, DCI, DAG);
   case AArch64ISD::TBNZ:
   case AArch64ISD::TBZ:
     return performTBZCombine(N, DCI, DAG);
   case AArch64ISD::CSEL:
     return performCSELCombine(N, DCI, DAG);
+  case AArch64ISD::ANDS:
+    return performANDSCombine(N, DCI);
   case AArch64ISD::DUP:
     return performPostLD1Combine(N, DCI, false);
   case AArch64ISD::NVCAST:
     return performNVCASTCombine(N);
   case AArch64ISD::SPLICE:
     return performSpliceCombine(N, DAG);
   case AArch64ISD::UUNPKLO:
   case AArch64ISD::UUNPKHI:

llvm/test/CodeGen/AArch64/peephole-and-tst.ll

 
 do.end:                                           ; preds = %4
   ret i32 %retval1.1
 }
 
 define i64 @test_and1(i64 %x, i64 %y) {
 ; CHECK-LABEL: test_and1:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    and x8, x0, #0x3
-; CHECK-NEXT:    tst x0, #0x3
+; CHECK-NEXT:    ands x8, x0, #0x3
 ; CHECK-NEXT:    csel x0, x8, x1, eq
 ; CHECK-NEXT:    ret
   %a = and i64 %x, 3
   %c = icmp eq i64 %a, 0
   %s = select i1 %c, i64 %a, i64 %y
   ret i64 %s
 }
 
 define i64 @test_and2(i64 %x, i64 %y) {
 ; CHECK-LABEL: test_and2:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    tst x0, #0x3
 ; CHECK-NEXT:    csel x0, x0, x1, eq
 ; CHECK-NEXT:    ret
   %a = and i64 %x, 3
   %c = icmp eq i64 %a, 0
   %s = select i1 %c, i64 %x, i64 %y
   ret i64 %s
 }
 
 define i64 @test_and3(i64 %x, i64 %y) {
 ; CHECK-LABEL: test_and3:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    stp x30, x21, [sp, #-32]! // 16-byte Folded Spill
+; CHECK-NEXT:    str x30, [sp, #-32]! // 8-byte Folded Spill
 ; CHECK-NEXT:    stp x20, x19, [sp, #16] // 16-byte Folded Spill
 ; CHECK-NEXT:    .cfi_def_cfa_offset 32
 ; CHECK-NEXT:    .cfi_offset w19, -8
 ; CHECK-NEXT:    .cfi_offset w20, -16
-; CHECK-NEXT:    .cfi_offset w21, -24
 ; CHECK-NEXT:    .cfi_offset w30, -32
 ; CHECK-NEXT:    mov x20, x0
-; CHECK-NEXT:    and x21, x0, #0x3
 ; CHECK-NEXT:    mov x0, xzr
 ; CHECK-NEXT:    mov x19, x1
 ; CHECK-NEXT:    bl callee
-; CHECK-NEXT:    tst x20, #0x3
-; CHECK-NEXT:    csel x0, x21, x19, eq
+; CHECK-NEXT:    ands x8, x20, #0x3
+; CHECK-NEXT:    csel x0, x8, x19, eq
 ; CHECK-NEXT:    ldp x20, x19, [sp, #16] // 16-byte Folded Reload
-; CHECK-NEXT:    ldp x30, x21, [sp], #32 // 16-byte Folded Reload
+; CHECK-NEXT:    ldr x30, [sp], #32 // 8-byte Folded Reload
 ; CHECK-NEXT:    ret
   %a = and i64 %x, 3
   %b = call i64 @callee(i64 0)
   %c = icmp eq i64 %a, 0
   %s = select i1 %c, i64 %a, i64 %y
   ret i64 %s
 }
 
 define i64 @test_and_4(i64 %x, i64 %y) {
 ; CHECK-LABEL: test_and_4:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    stp x30, x19, [sp, #-16]! // 16-byte Folded Spill
 ; CHECK-NEXT:    .cfi_def_cfa_offset 16
 ; CHECK-NEXT:    .cfi_offset w19, -8
 ; CHECK-NEXT:    .cfi_offset w30, -16
 ; CHECK-NEXT:    mov x19, x0
 ; CHECK-NEXT:    ands x0, x0, #0x3
 ; CHECK-NEXT:    bl callee
-; CHECK-NEXT:    tst x19, #0x3
-; CHECK-NEXT:    and x8, x19, #0x3
+; CHECK-NEXT:    ands x8, x19, #0x3
 ; CHECK-NEXT:    csel x0, x8, x0, eq
 ; CHECK-NEXT:    ldp x30, x19, [sp], #16 // 16-byte Folded Reload
 ; CHECK-NEXT:    ret
   %a = and i64 %x, 3
   %b = call i64 @callee(i64 %a)
   %c = icmp eq i64 %a, 0
   %s = select i1 %c, i64 %a, i64 %b
   ret i64 %s