[PATCH] D136352: [AArch64] Add SVE2.1 target feature for Armv9-A 2022 Architecture Extension

Paul Walker via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 20 10:47:48 PDT 2022


paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64.td:166
+def FeatureSVE2p1: SubtargetFeature<"sve2p1", "HasSVE2p1", "true",
+  "Enable Scalable Vector Extension 2.1  instructions", [FeatureSVE2]>;
+
----------------
Extra whitespace


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:159
                 "sve2 or sme">;
+def HasSVE2p1orSME
+    : Predicate<"Subtarget->hasSVE2p1() || Subtarget->hasSME()">,
----------------
I know the precedent has been set but I find this awkward to parse.  Is `HasSVE2p1_or_HasSME` a terrible idea? If agreeable we can always convert the other instance later on.


================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:3570
+defm PSEL_PPPRI : sve2_int_perm_sel_p<"psel", int_aarch64_sve_psel>;
+} // End let Predicates = [HasSVE2p1orSME]
----------------
Existing precedent is just `End HasSVE2p1orSME`.


================
Comment at: llvm/test/MC/AArch64/SME2/feature-sve2p1-implies-sve2.s:7
+cmla   z0.b, z1.b, z2.b, #0
+// CHECK-NOT: instruction requires: sve2 or sme
+// CHECK: cmla   z0.b, z1.b, z2.b, #0
----------------
Does the `CHECK-NOT` line provide any value? By which I mean such a message will be followed by an exit(1) and so the lit test will fail anyway?


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

https://reviews.llvm.org/D136352



More information about the llvm-commits mailing list