[PATCH] D135455: [AArch64] SME2 Single-multi vector ternary int/FP 2 and 4 registers

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64RegisterInfo.td:1372
 
+def MatrixIndexGPR32_8_11 : RegisterClass<"AArch64", [i32], 32, (sequence "W%u", 8, 11)>;
 def MatrixIndexGPR32_12_15 : RegisterClass<"AArch64", [i32], 32, (sequence "W%u", 12, 15)> {
----------------
Should this register class have `let DiagnosticType = "InvalidMatrixIndexGPR32_8_11"` with corresponding support in the switch-statements in AArch64AsmParser.cpp?


================
Comment at: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:3314
     {"sme-i64", {AArch64::FeatureSMEI64}},
+    {"sme2", {AArch64::FeatureSME2}},
     {"hbc", {AArch64::FeatureHBC}},
----------------
Should this be part of this patch? Or was this supposed to be part of D135448 ?


================
Comment at: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:4319-4320
   case AsmToken::Identifier: {
+    // See if this is a "VG" decoration used
+    // by SME instructions.
+    StringRef VecGroup;
----------------
nit: odd line-break (not at 80char limit)


================
Comment at: llvm/test/CodeGen/AArch64/stp-opt-with-renaming.mir:540
     liveins: $x0, $x1, $x8
-    renamable $x10, renamable $x9 = LDPXi renamable $x0, 0 :: (load (s64))
+    renamable undef $x10, renamable $x9 = LDPXi renamable $x0, 0 :: (load (s64))
     STRXui renamable killed $x9, renamable $x0, 11 :: (store (s64), align 4)
----------------
Why has the test itself changed here?


================
Comment at: llvm/test/CodeGen/AArch64/stp-opt-with-renaming.mir:367-371
+# CHECK-NEXT:    renamable $x9, renamable $x8 = LDPXi renamable $x0, 1 :: (load (s64))
+# CHECK-NEXT:    STRXui killed renamable $x9, renamable $x0, 11 :: (store (s64), align 4)
 # CHECK-NEXT:    renamable $x9 = LDRXui renamable $x0, 3 :: (load (s64))
 # CHECK-NEXT:    renamable $x14 = LDRXui renamable $x0, 5 :: (load (s64))
+# CHECK-NEXT:    STRXui renamable $x9, renamable $x0, 10 :: (store (s64), align 4)
----------------
aemerson wrote:
> The STP optimization isn't firing any more, this might be a problem with either the test or the optimization itself. IIRC @c-rhodes has looked into this before.
The issue seems to be caused by adding new register classes which seems to 'confuse' the AArch64LoadStoreOptimizer. @c-rhodes created D88663 a while back to fix this, but I haven't tried to see if that patch actually fixes the issue in this test. In any case the issue is exposed by this patch, not really caused by it (which can be seen by just adding the register class in isolation), so I think the issue should be fixed independently from this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135455