Allow GNU inline asm using target-specific dialect variant

[uweigand]

Summary

LLVM has two notions of "assembler dialect" that are conceptually distinct, but are currently somewhat intermixed.

The first notion corresponds to platform-specific syntactical differences in assembler text, e.g. the distinction between AT&T and Intel-style assembler for x86 (and similar differences on other architectures). The second notion corresponds to C/C++ source level differences in handling the inputs/outputs of *inline* assembler statements, specifically the difference between GNU inline asm and MS inline asm.

These are conceptually different, but the current inline assembler handling always enforces AT&T style assembler with GNU inline asm and Intel-style assembler with MS inline asm. This is done even on non-Intel platforms, which may not even have AT&T vs. Intel assembler -- that's not an issue for MS inline asm, as this itself only exists on Intel, but it is a problem if a platform wants to use different assembler styles for *GNU* inline asm. (Specifically, we want to do this on SystemZ to distinguish between GAS and HLASM style assembler text.)

This patch allows for more flexibility here by moving the "connection" between assembler styles and inline asm types into platform-specific code. Specifically, in addition to the existing "MAI->getAssemblerDialect" which returns the default style to be used for assembler reading and writing (which is currently uses everywhere *except* for inline asm), this patch introduces a "MAI->getInlineAsmDialect" which returns the assembler style to be used for parsing inline asm. The routine gets the inline asm type as type and therefore can return different styles for GNU and MS inline asm, which is then done in the X86 back-end.

[anirudhp]

Ping :)

[hubert.reinterpretcast]

llvm/include/llvm/MC/MCAsmInfo.h

572

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.

llvm/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.cpp

21–22

Why is this comment removed?

[rnk]

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.

llvm/include/llvm/MC/MCAsmInfo.h

572

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.

[anirudhp]

In D95444#2542739, @rnk wrote:

• 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

Yes this is the idea behind this patch and a series of subsequent patches (main RFC about introducing HLASM syntax):

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.

(I'm also commenting on this since I will be working on putting up some of the patches pertaining to the HLASM syntax soon.).

Would you please be able to expand on this a bit more? I don't believe we will allow intermixing of gnu asm and hlasm in an inline asm context anyway. Since the standard format of an HLASM statement (atleast the support that we're adding for it) has the same format of a gnu asm statement (with respect to its inputs, outputs, clobbers etc.). The dialect's "rules" comes into play while parsing the inline asm instructions into corresponding MCInst (like you mentioned) and have them emitted (either to object file or assembly file). The parsing of the input operands, output operands, clobbers remain unchanged. My apologies if I haven't seen your point you're trying to make right away.

[hubert.reinterpretcast]

In D95444#2542739, @rnk wrote:

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.

I'm concerned that "GCC" is overloaded here. The "GCC" that contrasts with MSVC is a C/C++ level parsing difference. The "GCC" that contrasts with HLASM has to do with the string content in a GNU-style inline assembly construct.

[rnk]

In D95444#2545646, @hubert.reinterpretcast wrote:

In D95444#2542739, @rnk wrote:

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.

I'm concerned that "GCC" is overloaded here. The "GCC" that contrasts with MSVC is a C/C++ level parsing difference. The "GCC" that contrasts with HLASM has to do with the string content in a GNU-style inline assembly construct.

Yes, sorry, I meant the assembly dialect that the GNU binutils assembler parses. What I'm trying to get at is that you might want to support separate blobs that use different dialects:

void foo() { asm volatile ("gnu style asm"); }
void bar() { asm volatile ("HLASM style asm"); }

If there is one global setting for the inline asm dialect, this won't work. You could create a flag to control the setting, but then if you want to use LTO on two objects that use different assembly dialects, the command line flag isn't sufficient. To fully address this use case, you would want to put the dialect on each inline assembly blob, similar to the way we handle the existing intel/gnu dialect flag.

[uweigand]

In D95444#2546235, @rnk wrote:

Yes, sorry, I meant the assembly dialect that the GNU binutils assembler parses. What I'm trying to get at is that you might want to support separate blobs that use different dialects:

void foo() { asm volatile ("gnu style asm"); }
void bar() { asm volatile ("HLASM style asm"); }

If there is one global setting for the inline asm dialect, this won't work. You could create a flag to control the setting, but then if you want to use LTO on two objects that use different assembly dialects, the command line flag isn't sufficient. To fully address this use case, you would want to put the dialect on each inline assembly blob, similar to the way we handle the existing intel/gnu dialect flag.

At least for our use case, this is not relevant. We want to use the "GCC style asm" whenever compiling for the z/OS target, and the "HLASM style asm" whenever compiling for the Linux target. These are incompatible anyway (different ABI, different target OS), so they cannot be combined using LTO. We simply need to use asm dialects since both targets are implemented in a single SystemZ back-end. (While the OS and ABI are different, the ISA is of course the same, so it does not make sense to use a different back-end.)

For this use case to work correctly, the back-end must be able to set the asm variant to be used for everything: writing asm source output, assembling a full asm source file, function-level inline asm, and module-level inline asm. There's really not much point in the front-end getting involved here, since the asm variant is already fully determined by the target triple.

Originally, I thought this would be a simple matter of the back-end setting the AssemblerDialect variable in its MCAsmInfo. However, it turned out this variable was actually only used for writing asm source output and assembling a full asm source file. For function-level inline asm, the dialect is taked from the IR, and for module-level inline asm, the dialect is hard-coded to 0 no matter what. This is what this patch was intended to fix.

Now, I fully agree that there are open questions on what the best implementation would be. Specifically, as Hubert points out above, the IR-level "dialect" field is currently overloaded with two semantics: it indicates both that the inline asm should be parsed using the MS-asm input/output syntax and that the Intel-style mnemonic/operand syntax should be used.

The idea of this patch is to explicitly decouple those two semantics by having the IR-level "dialect" only refer to the GCC vs. MS input/output syntax, and allowing the mnemonic/operand syntax to be always set by the back-end with no influence from the front-end / IR. In order to allow the X86 back-end to preserve existing behaviour, this meant that the back-end must be allowed to chose different mnemonic/operand dialects depending for GCC vs. MS input/output syntax inline asm statements, and therefore this is not a simple variable but a function.

If you have any better suggestions on how to implement this, I'd be happy to try those out as well ...

llvm/include/llvm/MC/MCAsmInfo.h

572

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.

llvm/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.cpp

21–22

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.

[anirudhp]

In D95444#2548271, @uweigand wrote:

At least for our use case, this is not relevant. We want to use the "GCC style asm" whenever compiling for the z/OS target, and the "HLASM style asm" whenever compiling for the Linux target. These are incompatible anyway (different ABI, different target OS), so they cannot be combined using LTO. We simply need to use asm dialects since both targets are implemented in a single SystemZ back-end. (While the OS and ABI are different, the ISA is of course the same, so it does not make sense to use a different back-end.)

@uweigand, its the other way right? We want to use the "GNU style asm" when compiling for the Linux target (on the Z backend) and use the "HLASM style asm" when compiling for the z/OS target (on the Z backend).

[uweigand]

In D95444#2548879, @anirudhp wrote:

In D95444#2548271, @uweigand wrote:

At least for our use case, this is not relevant. We want to use the "GCC style asm" whenever compiling for the z/OS target, and the "HLASM style asm" whenever compiling for the Linux target. These are incompatible anyway (different ABI, different target OS), so they cannot be combined using LTO. We simply need to use asm dialects since both targets are implemented in a single SystemZ back-end. (While the OS and ABI are different, the ISA is of course the same, so it does not make sense to use a different back-end.)

@uweigand, its the other way right? We want to use the "GNU style asm" when compiling for the Linux target (on the Z backend) and use the "HLASM style asm" when compiling for the z/OS target (on the Z backend).

Ah, of course, thanks for catching this. The rest of the argument remains unchanged, though :-)

[hubert.reinterpretcast]

llvm/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.cpp

21–22

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.

[uweigand]

llvm/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.cpp

21–22

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.

[hubert.reinterpretcast]

llvm/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.cpp

21–22

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.

[hubert.reinterpretcast]

llvm/include/llvm/MC/MCAsmInfo.h

572

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.

llvm/include/llvm/MC/MCAsmInfo.h

 // which are used in very similar ways across all targets.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_MC_MCASMINFO_H
 #define LLVM_MC_MCASMINFO_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/IR/InlineAsm.h"
 #include "llvm/MC/MCDirectives.h"
 #include "llvm/MC/MCTargetOptions.h"
 #include <vector>
 
 namespace llvm {
 
 class MCContext;
 class MCCFIInstruction;
StringRef getLinkerPrivateGlobalPrefix() const {
     return getPrivateGlobalPrefix();
   }
 
   const char *getInlineAsmStart() const { return InlineAsmStart; }
   const char *getInlineAsmEnd() const { return InlineAsmEnd; }
   const char *getCode16Directive() const { return Code16Directive; }
   const char *getCode32Directive() const { return Code32Directive; }
   const char *getCode64Directive() const { return Code64Directive; }
+  // Default assembler dialect for AsmParser and AsmWriter.
   unsigned getAssemblerDialect() const { return AssemblerDialect; }
+  // Allow overriding default dialect for inline assembler.
+  virtual unsigned getInlineAsmDialect(InlineAsm::AsmDialect asmDialect) const {
+    return getAssemblerDialect();
+  }
+
   bool doesAllowAtInName() const { return AllowAtInName; }
   bool doesAllowSymbolAtNameStart() const { return AllowSymbolAtNameStart; }
   bool supportsNameQuoting() const { return SupportsQuotedNames; }
 
   bool doesSupportDataRegionDirectives() const {
     return UseDataRegionDirectives;
   }
 

llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

void AsmPrinter::emitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
   // because it's not subtarget dependent.
   std::unique_ptr<MCInstrInfo> MII(TM.getTarget().createMCInstrInfo());
   assert(MII && "Failed to create instruction info");
   std::unique_ptr<MCTargetAsmParser> TAP(TM.getTarget().createMCAsmParser(
       STI, *Parser, *MII, MCOptions));
   if (!TAP)
     report_fatal_error("Inline asm not supported by this streamer because"
                        " we don't have an asm parser for this target\n");
-  Parser->setAssemblerDialect(Dialect);
+  Parser->setAssemblerDialect(MAI->getInlineAsmDialect(Dialect));
   Parser->setTargetParser(*TAP.get());
   // Enable lexing Masm binary and hex integer literals in intel inline
   // assembly.
   if (Dialect == InlineAsm::AD_Intel)
     Parser->getLexer().setLexMasmIntegers(true);
 
   emitInlineAsmStart();
   // Don't implicitly switch to the text section before the asm.
void AsmPrinter::emitInlineAsm(const MachineInstr *MI) const {
   }
 
   // Emit the inline asm to a temporary string so we can emit it through
   // EmitInlineAsm.
   SmallString<256> StringData;
   raw_svector_ostream OS(StringData);
 
   // The variant of the current asmprinter.
-  int AsmPrinterVariant = MAI->getAssemblerDialect();
+  int AsmPrinterVariant = MAI->getInlineAsmDialect(MI->getInlineAsmDialect());
   AsmPrinter *AP = const_cast<AsmPrinter*>(this);
   if (MI->getInlineAsmDialect() == InlineAsm::AD_ATT)
     EmitGCCInlineAsmStr(AsmStr, MI, MMI, AsmPrinterVariant, AP, LocCookie, OS);
   else
     EmitMSInlineAsmStr(AsmStr, MI, MMI, AP, LocCookie, OS);
 
   // Emit warnings if we use reserved registers on the clobber list, as
   // that might lead to undefined behaviour.

llvm/lib/Object/ModuleSymbolTable.cpp

initializeRecordStreamer(const Module &M,
 
   std::unique_ptr<MCTargetAsmParser> TAP(
       T->createMCAsmParser(*STI, *Parser, *MCII, MCOptions));
   if (!TAP)
     return;
 
   // Module-level inline asm is assumed to use At&t syntax (see
   // AsmPrinter::doInitialization()).
-  Parser->setAssemblerDialect(InlineAsm::AD_ATT);
+  Parser->setAssemblerDialect(MAI->getInlineAsmDialect(InlineAsm::AD_ATT));
 
   Parser->setTargetParser(*TAP);
   if (Parser->Run(false))
     return;
 
   Init(Streamer);
 }
 

llvm/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.h

 namespace llvm {
 class Triple;
 
 class X86MCAsmInfoDarwin : public MCAsmInfoDarwin {
   virtual void anchor();
 
 public:
   explicit X86MCAsmInfoDarwin(const Triple &Triple);
+  unsigned getInlineAsmDialect(InlineAsm::AsmDialect asmDialect) const override;
 };
 
 struct X86_64MCAsmInfoDarwin : public X86MCAsmInfoDarwin {
   explicit X86_64MCAsmInfoDarwin(const Triple &Triple);
   const MCExpr *
   getExprForPersonalitySymbol(const MCSymbol *Sym, unsigned Encoding,
                               MCStreamer &Streamer) const override;
 };
 
 class X86ELFMCAsmInfo : public MCAsmInfoELF {
   void anchor() override;
 
 public:
   explicit X86ELFMCAsmInfo(const Triple &Triple);
+  unsigned getInlineAsmDialect(InlineAsm::AsmDialect asmDialect) const override;
 };
 
 class X86MCAsmInfoMicrosoft : public MCAsmInfoMicrosoft {
   void anchor() override;
 
 public:
   explicit X86MCAsmInfoMicrosoft(const Triple &Triple);
+  unsigned getInlineAsmDialect(InlineAsm::AsmDialect asmDialect) const override;
 };
 
 class X86MCAsmInfoMicrosoftMASM : public X86MCAsmInfoMicrosoft {
   void anchor() override;
 
 public:
   explicit X86MCAsmInfoMicrosoftMASM(const Triple &Triple);
 };
 
 class X86MCAsmInfoGNUCOFF : public MCAsmInfoGNUCOFF {
   void anchor() override;
 
 public:
   explicit X86MCAsmInfoGNUCOFF(const Triple &Triple);
+  unsigned getInlineAsmDialect(InlineAsm::AsmDialect asmDialect) const override;
 };
 } // namespace llvm
 
 #endif

llvm/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.cpp

 #include "X86MCAsmInfo.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/Support/CommandLine.h"
 using namespace llvm;
 
 enum AsmWriterFlavorTy {
-  // Note: This numbering has to match the GCC assembler dialects for inline
-  // asm alternatives to work right.
   ATT = 0, Intel = 1
 };
 
+static enum AsmWriterFlavorTy X86InlineAsmDialect(InlineAsm::AsmDialect asmDialect) {
+  switch (asmDialect) {
+    case InlineAsm::AD_ATT: return ATT;
+    case InlineAsm::AD_Intel: return Intel;
+  }
+  llvm_unreachable("Invalid inline assembler dialect");
+}
+
 static cl::opt<AsmWriterFlavorTy> AsmWriterFlavor(
     "x86-asm-syntax", cl::init(ATT), cl::Hidden,
     cl::desc("Choose style of code to emit from X86 backend:"),
     cl::values(clEnumValN(ATT, "att", "Emit AT&T-style assembly"),
                clEnumValN(Intel, "intel", "Emit Intel-style assembly")));
 
 static cl::opt<bool>
 MarkedJTDataRegions("mark-data-regions", cl::init(true),
if (T.isMacOSX() && T.isMacOSXVersionLT(10, 6))
     HasWeakDefCanBeHiddenDirective = false;
 
   // Assume ld64 is new enough that the abs-ified FDE relocs may be used
   // (actually, must, since otherwise the non-extern relocations we produce
   // overwhelm ld64's tiny little mind and it fails).
   DwarfFDESymbolsUseAbsDiff = true;
 }
 
+unsigned X86MCAsmInfoDarwin::getInlineAsmDialect(InlineAsm::AsmDialect asmDialect) const {
+  return X86InlineAsmDialect(asmDialect);
+}
+
 X86_64MCAsmInfoDarwin::X86_64MCAsmInfoDarwin(const Triple &Triple)
   : X86MCAsmInfoDarwin(Triple) {
 }
 
 void X86ELFMCAsmInfo::anchor() { }
 
 X86ELFMCAsmInfo::X86ELFMCAsmInfo(const Triple &T) {
   bool is64Bit = T.getArch() == Triple::x86_64;
X86ELFMCAsmInfo::X86ELFMCAsmInfo(const Triple &T) {
 
   // Debug Information
   SupportsDebugInformation = true;
 
   // Exceptions handling
   ExceptionsType = ExceptionHandling::DwarfCFI;
 }
 
+unsigned X86ELFMCAsmInfo::getInlineAsmDialect(InlineAsm::AsmDialect asmDialect) const {
+  return X86InlineAsmDialect(asmDialect);
+}
+
 const MCExpr *
 X86_64MCAsmInfoDarwin::getExprForPersonalitySymbol(const MCSymbol *Sym,
                                                    unsigned Encoding,
                                                    MCStreamer &Streamer) const {
   MCContext &Context = Streamer.getContext();
   const MCExpr *Res =
     MCSymbolRefExpr::create(Sym, MCSymbolRefExpr::VK_GOTPCREL, Context);
   const MCExpr *Four = MCConstantExpr::create(4, Context);
X86MCAsmInfoMicrosoft::X86MCAsmInfoMicrosoft(const Triple &Triple) {
 
   AssemblerDialect = AsmWriterFlavor;
 
   TextAlignFillValue = 0x90;
 
   AllowAtInName = true;
 }
 
+unsigned X86MCAsmInfoMicrosoft::getInlineAsmDialect(InlineAsm::AsmDialect asmDialect) const {
+  return X86InlineAsmDialect(asmDialect);
+}
+
 void X86MCAsmInfoMicrosoftMASM::anchor() { }
 
 X86MCAsmInfoMicrosoftMASM::X86MCAsmInfoMicrosoftMASM(const Triple &Triple)
     : X86MCAsmInfoMicrosoft(Triple) {
   DollarIsPC = true;
   SeparatorString = "\n";
   CommentString = ";";
   AllowSymbolAtNameStart = true;
X86MCAsmInfoGNUCOFF::X86MCAsmInfoGNUCOFF(const Triple &Triple) {
   }
 
   AssemblerDialect = AsmWriterFlavor;
 
   TextAlignFillValue = 0x90;
 
   AllowAtInName = true;
 }
+
+unsigned X86MCAsmInfoGNUCOFF::getInlineAsmDialect(InlineAsm::AsmDialect asmDialect) const {
+  return X86InlineAsmDialect(asmDialect);
+}