[AArch64][SVE] Implement PFALSE with explicit AArch64ISD node.

[sdesmalen]

Summary

The ISel patterns for PFALSE helps recognise the instructions as being
free of side-effects, which helps MachineCSE remove redundant
PFALSE instructions.

[efriedma]

It looks like AArch64TargetLowering::LowerSPLAT_VECTOR is using DAG.getMachineNode in an inappropriate way: generally, in SelectionDAG, we shouldn't be creating Machine nodes before isel (AArch64DAGToDAGISel::Select). The "proper" construct is to define AArch64ISD::PFALSE, then add a pattern to lower it during isel.

The reason I bring this up is that hasSideEffects is usually inferred from isel patterns.

[sdesmalen]

Implement with AArch64ISD::PFALSE and patterns.

[sdesmalen]

In D118054#3267812, @efriedma wrote:

It looks like AArch64TargetLowering::LowerSPLAT_VECTOR is using DAG.getMachineNode in an inappropriate way: generally, in SelectionDAG, we shouldn't be creating Machine nodes before isel (AArch64DAGToDAGISel::Select). The "proper" construct is to define AArch64ISD::PFALSE, then add a pattern to lower it during isel.

The reason I bring this up is that hasSideEffects is usually inferred from isel patterns.

Ah that makes a lot of sense! I was a bit puzzled as to why it just worked for PTRUE, but not for PFALSE.
I've reimplemented it now by creating an explicit AArch64ISD node for it instead with patterns.

[dmgreen]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

2202–2203

Add a case here too, so it gets printed nicely in debug output.

llvm/lib/Target/AArch64/SVEInstrFormats.td

337

Does SDTCVecEltisVT imply SDTCisVec?

[sdesmalen]

Added CASE statement for pretty printing of node.

[sdesmalen]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

2202–2203

Good spot, cheers!

llvm/lib/Target/AArch64/SVEInstrFormats.td

337

Not sure, but when I add a pattern with i1 type, tblgen seems to go into an infinite loop.

[dmgreen]

Thanks. LGTM

llvm/lib/Target/AArch64/SVEInstrFormats.td

337

I think it should be fine without, from the use of SDTCVecEltisVT in other backends (and, you know, we only create AArch64ISD::PFALSE for vectors anyway..)

llvm/lib/Target/AArch64/AArch64ISelLowering.h

enum NodeType : unsigned {
   FMAXV_PRED,
   FMAXNMV_PRED,
   FMINV_PRED,
   FMINNMV_PRED,
 
   INSR,
   PTEST,
   PTRUE,
+  PFALSE,
 
   BITREVERSE_MERGE_PASSTHRU,
   BSWAP_MERGE_PASSTHRU,
   REVH_MERGE_PASSTHRU,
   REVW_MERGE_PASSTHRU,
   CTLZ_MERGE_PASSTHRU,
   CTPOP_MERGE_PASSTHRU,
   DUP_MERGE_PASSTHRU,

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

case AArch64ISD::FIRST_NUMBER:
     MAKE_CASE(AArch64ISD::ST2G)
     MAKE_CASE(AArch64ISD::STZ2G)
     MAKE_CASE(AArch64ISD::SUNPKHI)
     MAKE_CASE(AArch64ISD::SUNPKLO)
     MAKE_CASE(AArch64ISD::UUNPKHI)
     MAKE_CASE(AArch64ISD::UUNPKLO)
     MAKE_CASE(AArch64ISD::INSR)
     MAKE_CASE(AArch64ISD::PTEST)
     MAKE_CASE(AArch64ISD::PTRUE)
+    MAKE_CASE(AArch64ISD::PFALSE)
     MAKE_CASE(AArch64ISD::LD1_MERGE_ZERO)
     MAKE_CASE(AArch64ISD::LD1S_MERGE_ZERO)
     MAKE_CASE(AArch64ISD::LDNF1_MERGE_ZERO)
     MAKE_CASE(AArch64ISD::LDNF1S_MERGE_ZERO)
     MAKE_CASE(AArch64ISD::LDFF1_MERGE_ZERO)
     MAKE_CASE(AArch64ISD::LDFF1S_MERGE_ZERO)
     MAKE_CASE(AArch64ISD::LD1RQ_MERGE_ZERO)
     MAKE_CASE(AArch64ISD::LD1RO_MERGE_ZERO)
SDValue AArch64TargetLowering::LowerSPLAT_VECTOR(SDValue Op,
   // Extend input splat value where needed to fit into a GPR (32b or 64b only)
   // FPRs don't have this restriction.
   switch (ElemVT.getSimpleVT().SimpleTy) {
   case MVT::i1: {
     // The only legal i1 vectors are SVE vectors, so we can use SVE-specific
     // lowering code.
     if (auto *ConstVal = dyn_cast<ConstantSDNode>(SplatVal)) {
       if (ConstVal->isZero())
-        return SDValue(DAG.getMachineNode(AArch64::PFALSE, dl, VT), 0);
+        return DAG.getNode(AArch64ISD::PFALSE, dl, VT);
       if (ConstVal->isOne())
         return getPTrue(DAG, dl, VT, AArch64SVEPredPattern::all);
     }
     // The general case of i1.  There isn't any natural way to do this,
     // so we use some trickery with whilelo.
     SplatVal = DAG.getAnyExtOrTrunc(SplatVal, dl, MVT::i64);
     SplatVal = DAG.getNode(ISD::SIGN_EXTEND_INREG, dl, MVT::i64, SplatVal,
                            DAG.getValueType(MVT::i1));

llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td

let Predicates = [HasSVEorStreamingSVE] in {
   defm BRKA_PPzP  : sve_int_break_z<0b000, "brka",  int_aarch64_sve_brka_z>;
   defm BRKA_PPmP  : sve_int_break_m<0b001, "brka",  int_aarch64_sve_brka>;
   defm BRKAS_PPzP : sve_int_break_z<0b010, "brkas", null_frag>;
   defm BRKB_PPzP  : sve_int_break_z<0b100, "brkb",  int_aarch64_sve_brkb_z>;
   defm BRKB_PPmP  : sve_int_break_m<0b101, "brkb",  int_aarch64_sve_brkb>;
   defm BRKBS_PPzP : sve_int_break_z<0b110, "brkbs", null_frag>;
 
   def PTEST_PP : sve_int_ptest<0b010000, "ptest">;
-  def PFALSE   : sve_int_pfalse<0b000000, "pfalse">;
+  defm PFALSE  : sve_int_pfalse<0b000000, "pfalse">;
   defm PFIRST  : sve_int_pfirst<0b00000, "pfirst", int_aarch64_sve_pfirst>;
   defm PNEXT   : sve_int_pnext<0b00110, "pnext", int_aarch64_sve_pnext>;
 
   defm AND_PPzPP   : sve_int_pred_log<0b0000, "and", int_aarch64_sve_and_z, and>;
   defm BIC_PPzPP   : sve_int_pred_log<0b0001, "bic", int_aarch64_sve_bic_z>;
   defm EOR_PPzPP   : sve_int_pred_log<0b0010, "eor", int_aarch64_sve_eor_z, xor>;
   defm SEL_PPPP    : sve_int_pred_log<0b0011, "sel", vselect>;
   defm ANDS_PPzPP  : sve_int_pred_log<0b0100, "ands", null_frag>;

llvm/lib/Target/AArch64/SVEInstrFormats.td

multiclass sve_int_ptrue<bits<3> opc, string asm, SDPatternOperator op> {
   def : InstAlias<asm # "\t$Pd",
                   (!cast<Instruction>(NAME # _S) PPR32:$Pd, 0b11111), 1>;
   def : InstAlias<asm # "\t$Pd",
                   (!cast<Instruction>(NAME # _D) PPR64:$Pd, 0b11111), 1>;
 }
 
 def SDT_AArch64PTrue : SDTypeProfile<1, 1, [SDTCisVec<0>, SDTCisVT<1, i32>]>;
 def AArch64ptrue : SDNode<"AArch64ISD::PTRUE", SDT_AArch64PTrue>;
+def SDT_AArch64PFalse : SDTypeProfile<1, 0, [SDTCisVec<0>, SDTCVecEltisVT<0, i1>]>;
+def AArch64pfalse : SDNode<"AArch64ISD::PFALSE", SDT_AArch64PFalse>;
 
 let Predicates = [HasSVEorStreamingSVE] in {
   defm PTRUE  : sve_int_ptrue<0b000, "ptrue", AArch64ptrue>;
   defm PTRUES : sve_int_ptrue<0b001, "ptrues", null_frag>;
 }
 
 //===----------------------------------------------------------------------===//
 // SVE pattern match helpers.
: I<(outs PPR8:$Pd), (ins),
   let Inst{15-10} = 0b111001;
   let Inst{9}     = opc{0};
   let Inst{8-4}   = 0b00000;
   let Inst{3-0}   = Pd;
 
   let isReMaterializable = 1;
 }
 
+multiclass sve_int_pfalse<bits<6> opc, string asm> {
+  def NAME : sve_int_pfalse<opc, asm>;
+
+  def : Pat<(nxv16i1 (AArch64pfalse)), (!cast<Instruction>(NAME))>;
+  def : Pat<(nxv8i1 (AArch64pfalse)), (!cast<Instruction>(NAME))>;
+  def : Pat<(nxv4i1 (AArch64pfalse)), (!cast<Instruction>(NAME))>;
+  def : Pat<(nxv2i1 (AArch64pfalse)), (!cast<Instruction>(NAME))>;
+}
+
 class sve_int_ptest<bits<6> opc, string asm>
 : I<(outs), (ins PPRAny:$Pg, PPR8:$Pn),
   asm, "\t$Pg, $Pn",
   "",
   []>, Sched<[]> {
   bits<4> Pg;
   bits<4> Pn;
   let Inst{31-24} = 0b00100101;

llvm/test/CodeGen/AArch64/sve-pfalse-machine-cse.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -run-pass=machine-cse -mtriple=aarch64 -mattr=+sve -o - %s | FileCheck %s
+---
+name:            pfalse
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $p0
+
+    ; CHECK-LABEL: name: pfalse
+    ; CHECK: liveins: $p0
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:ppr = COPY $p0
+    ; CHECK-NEXT: [[PFALSE:%[0-9]+]]:ppr = PFALSE
+    ; CHECK-NEXT: [[UZP1_PPP_B:%[0-9]+]]:ppr = UZP1_PPP_B [[COPY]], [[PFALSE]]
+    ; CHECK-NEXT: [[UZP1_PPP_B1:%[0-9]+]]:ppr = UZP1_PPP_B killed [[UZP1_PPP_B]], [[PFALSE]]
+    ; CHECK-NEXT: $p0 = COPY [[UZP1_PPP_B1]]
+    ; CHECK-NEXT: RET_ReallyLR implicit $p0
+    %0:ppr = COPY $p0
+    %2:ppr = PFALSE
+    %3:ppr = UZP1_PPP_B %0, %2
+    %4:ppr = PFALSE
+    %5:ppr = UZP1_PPP_B killed %3, %4
+    $p0 = COPY %5
+    RET_ReallyLR implicit $p0
+...