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

Nico Weber via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 17 10:32:10 PST 2021


Author: Nico Weber
Date: 2021-11-17T13:31:59-05:00
New Revision: bf834b26292e58d86bc289a3eb0c736c400e600a

URL: https://github.com/llvm/llvm-project/commit/bf834b26292e58d86bc289a3eb0c736c400e600a
DIFF: https://github.com/llvm/llvm-project/commit/bf834b26292e58d86bc289a3eb0c736c400e600a.diff

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

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.)

Differential Revision: https://reviews.llvm.org/D113932

Added: 
    

Modified: 
    llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
    llvm/test/CodeGen/X86/asm-dialect.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
index 0c5f81bbebfc..2c53eade6cd5 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
@@ -129,13 +129,16 @@ void AsmPrinter::emitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
 }
 
 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) {
@@ -145,8 +148,8 @@ static void EmitMSInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
       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;
     }
@@ -164,6 +167,27 @@ static void EmitMSInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
       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;
 
@@ -183,8 +207,8 @@ static void EmitMSInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
         if (!StrEnd)
           report_fatal_error("Unterminated ${:foo} operand in inline asm"
                              " string: '" + Twine(AsmStr) + "'");
-
-        AP->PrintSpecial(MI, OS, StringRef(StrStart, StrEnd - StrStart));
+        if (CurVariant == -1 || CurVariant == AsmPrinterVariant)
+          AP->PrintSpecial(MI, OS, StringRef(StrStart, StrEnd - StrStart));
         LastEmitted = StrEnd+1;
         break;
       }
@@ -227,40 +251,42 @@ static void EmitMSInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
 
       // Okay, we finally have a value number.  Ask the target to print this
       // operand!
-      unsigned OpNo = InlineAsm::MIOp_FirstOperand;
+      if (CurVariant == -1 || CurVariant == AsmPrinterVariant) {
+        unsigned OpNo = InlineAsm::MIOp_FirstOperand;
 
-      bool Error = false;
+        bool Error = false;
 
-      // Scan to find the machine operand number for the operand.
-      for (; Val; --Val) {
-        if (OpNo >= MI->getNumOperands()) break;
-        unsigned OpFlags = MI->getOperand(OpNo).getImm();
-        OpNo += InlineAsm::getNumOperandRegisters(OpFlags) + 1;
-      }
+        // Scan to find the machine operand number for the operand.
+        for (; Val; --Val) {
+          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()) {
-        Error = true;
-      } else {
-        unsigned OpFlags = MI->getOperand(OpNo).getImm();
-        ++OpNo;  // Skip over the ID number.
-
-        if (InlineAsm::isMemKind(OpFlags)) {
-          Error = AP->PrintAsmMemoryOperand(
-              MI, OpNo, Modifier[0] ? Modifier : nullptr, OS);
+        // 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()) {
+          Error = true;
         } else {
-          Error = AP->PrintAsmOperand(MI, OpNo,
-                                      Modifier[0] ? Modifier : nullptr, OS);
+          unsigned OpFlags = MI->getOperand(OpNo).getImm();
+          ++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());
         }
-      }
-      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;
     }
@@ -273,7 +299,7 @@ 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();
@@ -290,7 +316,7 @@ static void EmitGCCInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
              *LiteralEnd != '}' && *LiteralEnd != '$' && *LiteralEnd != '\n')
         ++LiteralEnd;
       if (CurVariant == -1 || CurVariant == AsmPrinterVariant)
-        OS.write(LastEmitted, LiteralEnd-LastEmitted);
+        OS.write(LastEmitted, LiteralEnd - LastEmitted);
       LastEmitted = LiteralEnd;
       break;
     }
@@ -310,24 +336,24 @@ static void EmitGCCInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
           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;
@@ -401,7 +427,8 @@ static void EmitGCCInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
 
         // 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;
         }
@@ -409,12 +436,11 @@ static void EmitGCCInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
         // 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?
@@ -504,7 +530,7 @@ void AsmPrinter::emitInlineAsm(const MachineInstr *MI) const {
   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.

diff  --git a/llvm/test/CodeGen/X86/asm-dialect.ll b/llvm/test/CodeGen/X86/asm-dialect.ll
index 62d7fdad3d2e..ba1969dff4af 100644
--- a/llvm/test/CodeGen/X86/asm-dialect.ll
+++ b/llvm/test/CodeGen/X86/asm-dialect.ll
@@ -10,6 +10,11 @@ define void @f() {
 ; 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:
@@ -17,8 +22,13 @@ define void @f() {
 ; 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
 }


        


More information about the llvm-commits mailing list