[x86/asm] Let EmitMSInlineAsmStr() handle variants too

[thakis]

Summary

This is preparation for D113707, where I want to make -masm=intel
emit asm inteldialect instructions.

{movq %rbx, %rax|mov rax, rbx} is supposed to evaluate to the bit
between { and | for att and to the bit between | and } for intel.
Since intel will become asm inteldialect, which alls EmitMSInlineAsmStr(),
EmitMSInlineAsmStr() has to support variants as well.

(clang translates {...|...} to $(...$|...$). I'm not sure why
it doesn't just send along only the first ... or the second ...
to LLVM, but given the notes in PR23933 let's not do a big
reorganization in this codepath.)

This makes EmitMSInlineAsmStr() and EmitGCCInlineAsmStr()
even more alike.

[thakis]

rebase

[thakis]

clang-format

[hans]

lgtm

At what point does EmitGCCInlineAsmStr and EmitMSInlineAsmStr become similar enough that they could just take the desired variant as a parameter?

llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

141

Maybe this should be const?

[thakis]

In D113932#3137501, @hans wrote:

lgtm

At what point does EmitGCCInlineAsmStr and EmitMSInlineAsmStr become similar enough that they could just take the desired variant as a parameter?

That's an excellent question. I think we (I) should merge them again. Some of the remaining differences might be more bugs. I'll look into this.

llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

void AsmPrinter::emitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
   emitInlineAsmStart();
   // Don't implicitly switch to the text section before the asm.
   (void)Parser->Run(/*NoInitialTextSection*/ true,
                     /*NoFinalize*/ true);
   emitInlineAsmEnd(STI, &TAP->getSTI());
 }
 
 static void EmitMSInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
-                               MachineModuleInfo *MMI, AsmPrinter *AP,
-                               uint64_t LocCookie, raw_ostream &OS) {
+                               MachineModuleInfo *MMI, const MCAsmInfo *MAI,
+                               AsmPrinter *AP, uint64_t LocCookie,
+                               raw_ostream &OS) {
   // Switch to the inline assembly variant.
   OS << "\t.intel_syntax\n\t";
 
+  int CurVariant = -1; // The number of the {.|.|.} region we are in.
   const char *LastEmitted = AsmStr; // One past the last character emitted.
   unsigned NumOperands = MI->getNumOperands();
+  int AsmPrinterVariant = 1; // X86MCAsmInfo.cpp's AsmWriterFlavorTy::Intel.
 
   while (*LastEmitted) {
     switch (*LastEmitted) {
     default: {
       // Not a special case, emit the string section literally.
       const char *LiteralEnd = LastEmitted+1;
       while (*LiteralEnd && *LiteralEnd != '{' && *LiteralEnd != '|' &&
              *LiteralEnd != '}' && *LiteralEnd != '$' && *LiteralEnd != '\n')
         ++LiteralEnd;
-
-      OS.write(LastEmitted, LiteralEnd-LastEmitted);
+      if (CurVariant == -1 || CurVariant == AsmPrinterVariant)
+        OS.write(LastEmitted, LiteralEnd - LastEmitted);
       LastEmitted = LiteralEnd;
       break;
     }
     case '\n':
       ++LastEmitted;   // Consume newline character.
       OS << '\n';      // Indent code with newline.
       break;
     case '$': {
       ++LastEmitted;   // Consume '$' character.
       bool Done = true;
 
       // Handle escapes.
       switch (*LastEmitted) {
       default: Done = false; break;
       case '$':
         ++LastEmitted;  // Consume second '$' character.
         break;
+      case '(':        // $( -> same as GCC's { character.
+        ++LastEmitted; // Consume '(' character.
+        if (CurVariant != -1)
+          report_fatal_error("Nested variants found in inline asm string: '" +
+                             Twine(AsmStr) + "'");
+        CurVariant = 0; // We're in the first variant now.
+        break;
+      case '|':
+        ++LastEmitted; // Consume '|' character.
+        if (CurVariant == -1)
+          OS << '|'; // This is gcc's behavior for | outside a variant.
+        else
+          ++CurVariant; // We're in the next variant.
+        break;
+      case ')':        // $) -> same as GCC's } char.
+        ++LastEmitted; // Consume ')' character.
+        if (CurVariant == -1)
+          OS << '}'; // This is gcc's behavior for } outside a variant.
+        else
+          CurVariant = -1;
+        break;
       }
       if (Done) break;
 
       bool HasCurlyBraces = false;
       if (*LastEmitted == '{') {     // ${variable}
         ++LastEmitted;               // Consume '{' character.
         HasCurlyBraces = true;
       }
 
       // If we have ${:foo}, then this is not a real operand reference, it is a
       // "magic" string reference, just like in .td files.  Arrange to call
       // PrintSpecial.
       if (HasCurlyBraces && *LastEmitted == ':') {
         ++LastEmitted;
         const char *StrStart = LastEmitted;
         const char *StrEnd = strchr(StrStart, '}');
         if (!StrEnd)
           report_fatal_error("Unterminated ${:foo} operand in inline asm"
                              " string: '" + Twine(AsmStr) + "'");
-
+        if (CurVariant == -1 || CurVariant == AsmPrinterVariant)
           AP->PrintSpecial(MI, OS, StringRef(StrStart, StrEnd - StrStart));
         LastEmitted = StrEnd+1;
         break;
       }
 
       const char *IDStart = LastEmitted;
       const char *IDEnd = IDStart;
       while (isDigit(*IDEnd))
         ++IDEnd;
case '$': {
         if (*LastEmitted != '}')
           report_fatal_error("Bad ${} expression in inline asm string: '" +
                              Twine(AsmStr) + "'");
         ++LastEmitted;    // Consume '}' character.
       }
 
       // Okay, we finally have a value number.  Ask the target to print this
       // operand!
+      if (CurVariant == -1 || CurVariant == AsmPrinterVariant) {
         unsigned OpNo = InlineAsm::MIOp_FirstOperand;
 
         bool Error = false;
 
         // Scan to find the machine operand number for the operand.
         for (; Val; --Val) {
-        if (OpNo >= MI->getNumOperands()) break;
+          if (OpNo >= MI->getNumOperands())
+            break;
           unsigned OpFlags = MI->getOperand(OpNo).getImm();
           OpNo += InlineAsm::getNumOperandRegisters(OpFlags) + 1;
         }
 
         // We may have a location metadata attached to the end of the
         // instruction, and at no point should see metadata at any
         // other point while processing. It's an error if so.
-      if (OpNo >= MI->getNumOperands() ||
-          MI->getOperand(OpNo).isMetadata()) {
+        if (OpNo >= MI->getNumOperands() || MI->getOperand(OpNo).isMetadata()) {
           Error = true;
         } else {
           unsigned OpFlags = MI->getOperand(OpNo).getImm();
-        ++OpNo;  // Skip over the ID number.
+          ++OpNo; // Skip over the ID number.
 
           if (InlineAsm::isMemKind(OpFlags)) {
             Error = AP->PrintAsmMemoryOperand(
                 MI, OpNo, Modifier[0] ? Modifier : nullptr, OS);
           } else {
             Error = AP->PrintAsmOperand(MI, OpNo,
                                         Modifier[0] ? Modifier : nullptr, OS);
           }
         }
         if (Error) {
           std::string msg;
           raw_string_ostream Msg(msg);
           Msg << "invalid operand in inline asm: '" << AsmStr << "'";
           MMI->getModule()->getContext().emitError(LocCookie, Msg.str());
         }
+      }
       break;
     }
     }
   }
   OS << "\n\t.att_syntax\n" << (char)0;  // null terminate string.
 }
 
 static void EmitGCCInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
                                 MachineModuleInfo *MMI, const MCAsmInfo *MAI,
                                 AsmPrinter *AP, uint64_t LocCookie,
                                 raw_ostream &OS) {
-  int CurVariant = -1;            // The number of the {.|.|.} region we are in.
+  int CurVariant = -1; // The number of the {.|.|.} region we are in.
   const char *LastEmitted = AsmStr; // One past the last character emitted.
   unsigned NumOperands = MI->getNumOperands();
   int AsmPrinterVariant = MMI->getTarget().unqualifiedInlineAsmVariant();
 
   if (MAI->getEmitGNUAsmStartIndentationMarker())
     OS << '\t';
 
   while (*LastEmitted) {
     switch (*LastEmitted) {
     default: {
       // Not a special case, emit the string section literally.
       const char *LiteralEnd = LastEmitted+1;
       while (*LiteralEnd && *LiteralEnd != '{' && *LiteralEnd != '|' &&
              *LiteralEnd != '}' && *LiteralEnd != '$' && *LiteralEnd != '\n')
         ++LiteralEnd;
       if (CurVariant == -1 || CurVariant == AsmPrinterVariant)
-        OS.write(LastEmitted, LiteralEnd-LastEmitted);
+        OS.write(LastEmitted, LiteralEnd - LastEmitted);
       LastEmitted = LiteralEnd;
       break;
     }
     case '\n':
       ++LastEmitted;   // Consume newline character.
       OS << '\n';      // Indent code with newline.
       break;
     case '$': {
       ++LastEmitted;   // Consume '$' character.
       bool Done = true;
 
       // Handle escapes.
       switch (*LastEmitted) {
       default: Done = false; break;
       case '$':     // $$ -> $
         if (CurVariant == -1 || CurVariant == AsmPrinterVariant)
           OS << '$';
         ++LastEmitted;  // Consume second '$' character.
         break;
-      case '(':             // $( -> same as GCC's { character.
-        ++LastEmitted;      // Consume '(' character.
+      case '(':        // $( -> same as GCC's { character.
+        ++LastEmitted; // Consume '(' character.
         if (CurVariant != -1)
           report_fatal_error("Nested variants found in inline asm string: '" +
                              Twine(AsmStr) + "'");
-        CurVariant = 0;     // We're in the first variant now.
+        CurVariant = 0; // We're in the first variant now.
         break;
       case '|':
-        ++LastEmitted;  // consume '|' character.
+        ++LastEmitted; // Consume '|' character.
         if (CurVariant == -1)
-          OS << '|';       // this is gcc's behavior for | outside a variant
+          OS << '|'; // This is gcc's behavior for | outside a variant.
         else
-          ++CurVariant;   // We're in the next variant.
+          ++CurVariant; // We're in the next variant.
         break;
-      case ')':         // $) -> same as GCC's } char.
-        ++LastEmitted;  // consume ')' character.
+      case ')':        // $) -> same as GCC's } char.
+        ++LastEmitted; // Consume ')' character.
         if (CurVariant == -1)
-          OS << '}';     // this is gcc's behavior for } outside a variant
+          OS << '}'; // This is gcc's behavior for } outside a variant.
         else
           CurVariant = -1;
         break;
       }
       if (Done) break;
 
       bool HasCurlyBraces = false;
       if (*LastEmitted == '{') {     // ${variable}
case '$': {
       // operand!
       if (CurVariant == -1 || CurVariant == AsmPrinterVariant) {
         unsigned OpNo = InlineAsm::MIOp_FirstOperand;
 
         bool Error = false;
 
         // Scan to find the machine operand number for the operand.
         for (; Val; --Val) {
-          if (OpNo >= MI->getNumOperands()) break;
+          if (OpNo >= MI->getNumOperands())
+            break;
           unsigned OpFlags = MI->getOperand(OpNo).getImm();
           OpNo += InlineAsm::getNumOperandRegisters(OpFlags) + 1;
         }
 
         // We may have a location metadata attached to the end of the
         // instruction, and at no point should see metadata at any
         // other point while processing. It's an error if so.
-        if (OpNo >= MI->getNumOperands() ||
-            MI->getOperand(OpNo).isMetadata()) {
+        if (OpNo >= MI->getNumOperands() || MI->getOperand(OpNo).isMetadata()) {
           Error = true;
         } else {
           unsigned OpFlags = MI->getOperand(OpNo).getImm();
-          ++OpNo;  // Skip over the ID number.
+          ++OpNo; // Skip over the ID number.
 
           // FIXME: Shouldn't arch-independent output template handling go into
           // PrintAsmOperand?
           // Labels are target independent.
           if (MI->getOperand(OpNo).isBlockAddress()) {
             const BlockAddress *BA = MI->getOperand(OpNo).getBlockAddress();
             MCSymbol *Sym = AP->GetBlockAddressSymbol(BA);
             Sym->print(OS, AP->MAI);
void AsmPrinter::emitInlineAsm(const MachineInstr *MI) const {
   // EmitInlineAsm.
   SmallString<256> StringData;
   raw_svector_ostream OS(StringData);
 
   AsmPrinter *AP = const_cast<AsmPrinter*>(this);
   if (MI->getInlineAsmDialect() == InlineAsm::AD_ATT)
     EmitGCCInlineAsmStr(AsmStr, MI, MMI, MAI, AP, LocCookie, OS);
   else
-    EmitMSInlineAsmStr(AsmStr, MI, MMI, AP, LocCookie, OS);
+    EmitMSInlineAsmStr(AsmStr, MI, MMI, MAI, AP, LocCookie, OS);
 
   // Emit warnings if we use reserved registers on the clobber list, as
   // that might lead to undefined behaviour.
   SmallVector<Register, 8> RestrRegs;
   const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
   // Start with the first operand descriptor, and iterate over them.
   for (unsigned I = InlineAsm::MIOp_FirstOperand, NumOps = MI->getNumOperands();
        I < NumOps; ++I) {

llvm/test/CodeGen/X86/asm-dialect.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=x86_64-unknown-linux-gnu %s -o - \
 ; RUN:     | FileCheck --check-prefix=OUTPUT_ATT %s
 ; RUN: llc -mtriple=x86_64-unknown-linux-gnu %s -x86-asm-syntax=intel -o - \
 ; RUN:     | FileCheck --check-prefix=OUTPUT_INTEL %s
 
 define void @f() {
 ; OUTPUT_ATT-LABEL: f:
 ; OUTPUT_ATT:       # %bb.0:
 ; OUTPUT_ATT-NEXT:    #APP
 ; OUTPUT_ATT-NEXT:    movq %rbx, %rax
 ; OUTPUT_ATT-NEXT:    #NO_APP
+; OUTPUT_ATT-NEXT:    #APP
+; OUTPUT_ATT-EMPTY:
+; OUTPUT_ATT-NEXT:    movq %rbx, %rax
+; OUTPUT_ATT-EMPTY:
+; OUTPUT_ATT-NEXT:    #NO_APP
 ; OUTPUT_ATT-NEXT:    retq
 ;
 ; OUTPUT_INTEL-LABEL: f:
 ; OUTPUT_INTEL:       # %bb.0:
 ; OUTPUT_INTEL-NEXT:    #APP
 ; OUTPUT_INTEL-NEXT:    mov rax, rbx
 ; OUTPUT_INTEL-NEXT:    #NO_APP
+; OUTPUT_INTEL-NEXT:    #APP
+; OUTPUT_INTEL-EMPTY:
+; OUTPUT_INTEL-NEXT:    mov rax, rbx
+; OUTPUT_INTEL-EMPTY:
+; OUTPUT_INTEL-NEXT:    #NO_APP
 ; OUTPUT_INTEL-NEXT:    ret
   call void asm sideeffect "$(movq %rbx, %rax $|mov rax, rbx$)", "~{dirflag},~{fpsr},~{flags}"()
-
+  call void asm sideeffect inteldialect "$(movq %rbx, %rax $|mov rax, rbx$)", "~{dirflag},~{fpsr},~{flags}"()
   ret void
 }