[PATCH] D112420: [clang][ARM] PACBTI-M assembly support

[Oliver Stannard (Linaro) via Phabricator via cfe-commits]

ostannard added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMInstrThumb2.td:4083
 def : t2InstAlias<"csdb$p",   (t2HINT 20, pred:$p), 1>;
+def : t2InstAlias<"pacbti$p r12,lr,sp", (t2HINT 13, pred:$p), 1>;
+def : t2InstAlias<"bti$p", (t2HINT 15, pred:$p), 1>;
----------------
Why are these needed in addition to the PACBTIHintSpaceInst instructions below?


================
Comment at: llvm/lib/Target/ARM/ARMInstrThumb2.td:5674
+
+def t2AUTG  : PACBTIAut<"autg", 0>;
+def t2BXAUT : PACBTIAut<"bxaut", 1>;
----------------
I think all of the `AUT` instructions need `hasSideEffects` set, since they can raise exceptions.


================
Comment at: llvm/lib/Target/ARM/ARMInstrThumb2.td:5675
+def t2AUTG  : PACBTIAut<"autg", 0>;
+def t2BXAUT : PACBTIAut<"bxaut", 1>;
+}
----------------
This needs `isBranch` set, and a test added to test/MC/ARM/implicit-it-generation.s.


================
Comment at: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:6441
+      Mnemonic == "csetm" ||
+      Mnemonic == "autg"  || Mnemonic == "aut"   ||
+      Mnemonic == "bxaut" || Mnemonic == "pacg"  || Mnemonic == "pac" ||
----------------
PACG, AUTG and BXAUT can be conditional, so shouldn't be in this list.


================
Comment at: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:6590
+      Mnemonic.startswith("vpt") || Mnemonic.startswith("vpst") ||
+      Mnemonic == "pacg" || Mnemonic == "pac" || Mnemonic == "pacbti" ||
+      Mnemonic == "autg" || Mnemonic == "aut" || Mnemonic == "bxaut" ||
----------------
Again, some of these are predicable.


================
Comment at: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:12283
       // FIXME: Unsupported extensions.
-      {ARM::AEK_OS, {}, {}},
+{ARM::AEK_OS, {}, {}},
       {ARM::AEK_IWMMXT, {}, {}},
----------------
Unintentional formatting change.


================
Comment at: llvm/test/MC/ARM/armv8.1m-pacbti-error.s:3
+
+// CHECK: error: invalid instruction
+pac r0, r1, r2
----------------
We should also test the cases where PACG/AUTG/BXAUT cannot use PC/SP.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112420