[PATCH] D154084: [AArch64] Modify SVE Pseudo appends

[Ricardo Jesus via Phabricator via llvm-commits]

rjj added a comment.

This looks like a nice patch. I left a few comments for the Neoverse V2 below.

Also, though not particularly important for now, maybe we could also normalise SME instructions (like `ADDHA_MPPZ_D_PSEUDO_D`)?



================
Comment at: llvm/lib/Target/AArch64/AArch64SchedNeoverseV2.td:2081-2082
 def : InstRW<[V2Write_2cyc_1V],
-             (instregex "^(ABS|ADD|CNOT|NEG|SUB|SUBR)_ZPmZ_[BHSD]$",
-                        "^(ABS|CNOT|NEG)_ZPmZ_UNDEF_[BHSD]$",
-                        "^(ADD|SUB)_ZZZ_[BHSD]$",
-                        "^(ADD|SUB|SUBR)_ZI_[BHSD]$",
-                        "^ADR_[SU]XTW_ZZZ_D_[0123]$",
-                        "^ADR_LSL_ZZZ_[SD]_[0123]$",
-                        "^[SU](ADD|SUB)[LW][BT]_ZZZ_[HSD]$",
-                        "^SADDLBT_ZZZ_[HSD]$",
-                        "^[SU]H(ADD|SUB|SUBR)_ZPmZ_[BHSD]$",
-                        "^SSUBL(BT|TB)_ZZZ_[HSD]$")>;
+             (instregex "^(ABS|ADD|CNOT|NEG|SUB|SUBR)_ZPmZ_[BHSD]",
+                        "^(ABS|CNOT|NEG)_ZPmZ_[BHSD]",
+                        "^(ADD|SUB)_ZZZ_[BHSD]",
----------------
This line is actually not necessary anymore


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedNeoverseV2.td:2095-2096
+             (instregex "^R?(ADD|SUB)HN[BT]_ZZZ_[BHS]",
+                        "^SQ(ABS|ADD|NEG|SUB|SUBR)_ZPmZ_[BHSD]",
+                        "^SQ(ABS|NEG)_ZPmZ_[BHSD]",
+                        "^[SU]Q(ADD|SUB)_ZZZ_[BHSD]",
----------------
Same as above.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedNeoverseV2.td:2156-2157
 // Count/reverse bits
-def : InstRW<[V2Write_2cyc_1V], (instregex "^(CLS|CLZ|CNT|RBIT)_ZPmZ_[BHSD]$",
-                                           "^(CLS|CLZ|CNT)_ZPmZ_UNDEF_[BHSD]$")>;
+def : InstRW<[V2Write_2cyc_1V], (instregex "^(CLS|CLZ|CNT|RBIT)_ZPmZ_[BHSD]",
+                                           "^(CLS|CLZ|CNT)_ZPmZ_[BHSD]")>;
 
----------------
Unnecessary now.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedNeoverseV2.td:2370-2371
 // Reciprocal estimate
 def : InstRW<[V2Write_4cyc_2V02], (instrs URECPE_ZPmZ_S, URSQRTE_ZPmZ_S,
-                                          URECPE_ZPmZ_UNDEF_S, URSQRTE_ZPmZ_UNDEF_S)>;
+                                          URECPE_ZPmZ_S, URSQRTE_ZPmZ_S)>;
 
----------------
These should still use the explicit forms, or be changed to instregex.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedNeoverseV2.td:2524-2525
 // Floating point reciprocal estimate, F16
 def : InstRW<[V2Write_6cyc_4V02], (instrs FRECPE_ZZ_H, FRECPX_ZPmZ_H,
-                                          FRSQRTE_ZZ_H, FRECPX_ZPmZ_UNDEF_H)>;
+                                          FRSQRTE_ZZ_H, FRECPX_ZPmZ_H)>;
 
----------------
Should still use the explicit form.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedNeoverseV2.td:2528-2529
 // Floating point reciprocal estimate, F32
 def : InstRW<[V2Write_4cyc_2V02], (instrs FRECPE_ZZ_S, FRECPX_ZPmZ_S,
-                                          FRSQRTE_ZZ_S, FRECPX_ZPmZ_UNDEF_S)>;
+                                          FRSQRTE_ZZ_S, FRECPX_ZPmZ_S)>;
 
----------------
Ditto.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedNeoverseV2.td:2533
 def : InstRW<[V2Write_3cyc_1V02], (instrs FRECPE_ZZ_D, FRECPX_ZPmZ_D,
-                                          FRSQRTE_ZZ_D, FRECPX_ZPmZ_UNDEF_D)>;
+                                          FRSQRTE_ZZ_D, FRECPX_ZPmZ_D)>;
 
----------------
Ditto.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedNeoverseV2.td:2560
 // Floating point square root, F16
-def : InstRW<[V2Write_13cyc_1V0_12rc], (instrs FSQRT_ZPmZ_H, FSQRT_ZPmZ_UNDEF_H)>;
+def : InstRW<[V2Write_13cyc_1V0_12rc], (instrs FSQRT_ZPmZ_H, FSQRT_ZPmZ_H)>;
 
----------------
Should use the explicit form.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedNeoverseV2.td:2563
 // Floating point square root, F32
-def : InstRW<[V2Write_10cyc_1V0_9rc], (instrs FSQRT_ZPmZ_S, FSQRT_ZPmZ_UNDEF_S)>;
+def : InstRW<[V2Write_10cyc_1V0_9rc], (instrs FSQRT_ZPmZ_S, FSQRT_ZPmZ_S)>;
 
----------------
Ditto.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedNeoverseV2.td:2566
 // Floating point square root, F64
-def : InstRW<[V2Write_16cyc_1V0_14rc], (instrs FSQRT_ZPmZ_D, FSQRT_ZPmZ_UNDEF_D)>;
+def : InstRW<[V2Write_16cyc_1V0_14rc], (instrs FSQRT_ZPmZ_D, FSQRT_ZPmZ_D)>;
 
----------------
Ditto.


================
Comment at: llvm/unittests/Target/AArch64/AArch64SvePseudoTest.cpp:49
+    // Check if instruction is in the pseudo table
+    // i holds the opcode of the pseudo, orgiInstr holds the opcode of the
+    // original instruction
----------------
nit: typo


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154084/new/

https://reviews.llvm.org/D154084