[PATCH] D155896: [Target][MC] Cleaning up AssemblerDialect / InstructionPrinterSyntaxVariant

Christoph Stiller via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 20 18:57:02 PDT 2023

rainerzufalldererste added inline comments.

Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp:120-137
+  // Resolve `InlineAsm::AsmDialect` to `AsmDialect`.
+  AsmDialect::Type CorrespondingAsmDialect;
+  switch (Dialect) {
+  case InlineAsm::AD_ATT:
+    CorrespondingAsmDialect = AsmDialect::X86_ATT;
+    break;
This is where I expected to be a good place for a point of separation - but my initial version also transitioned all of InlineAsm to the new enum.
I'm not sure how the inline assembly is supported across different target platforms, so please let me know whether that separation is justified.

Comment at: llvm/lib/MC/MCDisassembler/Disassembler.cpp:324-325
+      AsmDialect::Type AsmPrinterVariant = MAI->getAssemblerDialect();
+      // what is the following line supposed to achieve? Do we want this?
+      AsmPrinterVariant = (AsmDialect::Type)(AsmPrinterVariant == 0 ? 1 : 0);
       MCInstPrinter *IP = DC->getTarget()->createMCInstPrinter(
The comment obviously needs to be removed, but I wasn't sure what the line is trying to achieve and there's probably a cleaner way to do it now.

Comment at: llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCAsmInfo.h:18
 class Triple;
-enum SystemZAsmDialect { AD_ATT = 0, AD_HLASM = 1 };
Would you prefer to keep this around, even through it's now unused (for potential backwards-compatibility)?

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list