[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/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.cpp:21-22
 enum AsmWriterFlavorTy {
-  // Note: This numbering has to match the GCC assembler dialects for inline
-  // asm alternatives to work right.
   ATT = 0, Intel = 1
----------------
uweigand wrote:
> hubert.reinterpretcast wrote:
> > uweigand wrote:
> > > hubert.reinterpretcast wrote:
> > > > Why is this comment removed?
> > > Well, the whole reason the numbering had to match was that the value is currently overloaded between the "GNU vs. MS" values and "AT&T vs Intel" values.  With the patch, the overload is no longer present (the correspondence is rather explicitly provided via a function), and therefore the numbers no longer have to match.
> > This patch provides a correspondence from some input //to// these values. I am not seeing where the consumers (i.e., via `AsmWriterFlavor` and `AssemblerDialect`, the callers of `getAssemblerDialect`) are being updated by this patch to not be sensitive to the specific numbering here.
> > 
> It used to be the case that those numbers had to match the values of the AsmDialect enum (i.e. ATT == AsmDialect::AD_ATT and Intel == AsmDialect::AD_Intel).  This is what I interpreted the comment to refer to, and that no longer holds true.
> 
> The only consumers of that value should be now the interpretation of {..|..} variants in assembler strings.  Of course it is still the case that the variant numbers cannot be arbitrarily changed if inline asm statements in existing source code already use the {..|..} syntax.  If that's what the comment is supposed to refer to, then it would remain true.
Yes, that syntax is what I interpret the comment to refer to. The `EmitGCCInlineAsmStr` function operates using these values as arguments to its `AsmPrinterVariant` parameter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95444