[PATCH] D95444: Allow GNU inline asm using target-specific dialect variant

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

Sorry, I got a bit lost initially thinking about MS/Intel/GNU inline asm. The main motivation for this patch is SystemZ HLASM vs GNU asm. The idea behind this patch is that:

- LLVM will continue to emit GNU-style assembly on SystemZ (non-inline writing case)
- LLVM will continue to parse .s files in the GNU dialect by default (non-inline reading case)
- Inline asm will now be parsed into MCInstructions using the HLASM dialect, and then the MCInstructions will either be directly emitted or textually emitted in the GNU dialect

That seems reasonable, but I wonder if the change should be in the frontend. Clang actually supports mixing GCC and MSVC inline asm in the same translation unit, and it uses the dialect flag in the IR to control this (I think). This is important if you want to support LTO between two TUs that use GCC and HLASM. Maybe there is exactly zero inline asm that uses GCC-style asm for SystemZ, so maybe this isn't a concern, but I thought I'd ask.



================
Comment at: llvm/include/llvm/MC/MCAsmInfo.h:572
+  // Allow overriding default dialect for inline assembler.
+  virtual unsigned getInlineAsmDialect(InlineAsm::AsmDialect asmDialect) const {
+    return getAssemblerDialect();
----------------
hubert.reinterpretcast wrote:
> The parameter here needs to be explained more clearly. This function seems to be compensating for problems in the applicability/correctness of what the input argument represents. I think a function that just actually gives a default dialect for the inline assembly would be easier to understand. I guess that in cases where the input to this function matters, we should consider changing the code where the input argument was originally set.
Can this be a field instead of a virtual method? Most MCAsmInfo customization points seem to be data instead of code, and I think it's usually easier to reason about data than code. I see that this method has a parameter and that the x86 implementations use it, and I'm not immediately sure how to replace that, but it seems like there might be a way to do it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95444