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

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
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();
----------------
uweigand wrote:
> rnk wrote:
> > 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.
> The intent is not to "compensate for problems", really, but to clearly separate the two notions of "GNU vs. MS input/output syntax" from "AT&T vs. Intel mnemonic/operand syntax".  This function maps a value from the first domain into one of the second domain, allowing the back-end to provide the appropriate default mnemonic/operand syntax depending on the GNU vs. MS inline asm statement.
> The intent is [ ... ] to clearly separate the two notions of "GNU vs. MS input/output syntax" from "AT&T vs. Intel mnemonic/operand syntax".

Some renaming of the enumerators in the first domain would aid comprehension and make this patch easier to move forward.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95444