[PATCH] D137571: [AArch64] Add all SME2.1 instructions Assembly/Disassembly

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added a comment.

I've only glanced at the patch so far (will take a proper look tomorrow) but I've noticed what looks like a couple of rebase issues.



================
Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:133
 AARCH64_ARCH_EXT_NAME("b16b16",       AArch64::AEK_B16B16,      "+b16b16",       "-b16b16")
+AARCH64_ARCH_EXT_NAME("sme2p1",       AArch64::AEK_SME2p1,      "+sme2p1",       "-sme2p1")
+AARCH64_ARCH_EXT_NAME("sme-f16f16",   AArch64::AEK_SMEF16F16,   "+sme-f16f16",   "-sme-f16f16")
----------------
This already exists further down.


================
Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:134
+AARCH64_ARCH_EXT_NAME("sme2p1",       AArch64::AEK_SME2p1,      "+sme2p1",       "-sme2p1")
+AARCH64_ARCH_EXT_NAME("sme-f16f16",   AArch64::AEK_SMEF16F16,   "+sme-f16f16",   "-sme-f16f16")
 AARCH64_ARCH_EXT_NAME("rcpc",         AArch64::AEK_RCPC,        "+rcpc",         "-rcpc")
----------------
Can you move this to just before `sme-f64f64`?


================
Comment at: llvm/include/llvm/Support/AArch64TargetParser.h:77-78
   AEK_SVE2p1 =      1ULL << 44, // FEAT_SVE2p1
-  AEK_SME2p1 =      1ULL << 45, // FEAT_SME2p1
-  AEK_B16B16 =      1ULL << 46  // FEAT_B16B16
+  AEK_B16B16 =      1ULL << 45, // FEAT_B16B16
+  AEK_SME2p1 =      1ULL << 46, // FEAT_SME2p1
+  AEK_SMEF16F16 =   1ULL << 47, // FEAT_SMEF16F16
----------------
It looks like you've inadvertently switched these during a rebase.


================
Comment at: llvm/lib/Target/AArch64/AArch64.td:174-175
 
+def FeatureSMEF16F16 : SubtargetFeature<"sme-f16f16", "HasSMEF16F16", "true",
+  "Enable SME2.1 non-widening Float16 instructions (FEAT_SME-F16F16)", []>;
+
----------------
Please move this so it sits nearer the other SME feature flags.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:153-154
                                  AssemblerPredicateWithAll<(all_of FeatureSME2p1), "sme2p1">;
+def HasSMEF16F16     : Predicate<"Subtarget->hasSMEF16F16()">,
+                                 AssemblerPredicateWithAll<(all_of FeatureSMEF16F16), "sme-f16f16">;
+
----------------
Please move this just before `HasSMEF64F64`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137571