[PATCH] D89138: [AArch64] Implement .variant_pcs directive

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 9 10:14:21 PDT 2020


sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:817
+      STI->getRegisterInfo()->hasSVEArgsOrReturn(MF)) {
+    AArch64TargetStreamer *TS =
+        static_cast<AArch64TargetStreamer *>(OutStreamer->getTargetStreamer());
----------------
nit: `auto` ?


================
Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:821
+    OutStreamer->emitLabel(CurrentFnSym);
+    return;
+  }
----------------
Should this branch also call `emitFunctionEntryLabel()` ?


================
Comment at: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:5671
+  if (!Sym)
+    return false;
+
----------------
Should this give an error?


================
Comment at: llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFStreamer.cpp:50
 
+  void emitDirectiveVariantPCS(MCSymbol *Symbol) override {
+    OS << "\t.variant_pcs " << Symbol->getName() << "\n";
----------------
nit: `const MCSymbol *Symbol`


================
Comment at: llvm/test/CodeGen/AArch64/variant-pcs.ll:53
+
+define aarch64_sve_vector_pcs <vscale x 4 x i32> @sve_vector_pcs_5(<vscale x 4 x i32> %arg) {
+; CHECK-ASM: .variant_pcs sve_vector_pcs_5
----------------
nit: I think this test can be removed, as we want to remove `aarch64_sve_vector_pcs` (because it is implied by the operands to the function when using the default cc).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89138



More information about the llvm-commits mailing list