[ms] [X86] Use "P" modifier on operands to call instructions in inline X86 assembly.

Summary:
This is documented as the appropriate template modifier for call operands.
Fixes PR44272, and adds a regression test.

Also adds support for operand modifiers in Intel-style inline assembly.

Reviewers: rnk

Reviewed By: rnk

Subscribers: merge_guards_bot, hiraditya, cfe-commits, llvm-commits

Tags: #clang, #llvm

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

Author: ericastor

clang/lib/CodeGen/TargetInfo.cpp

@@ -1177,6 +1177,10 @@ static void rewriteInputConstraintReferences(unsigned FirstIn,
     if (NumDollars % 2 != 0 && Pos < AsmString.size()) {
       // We have an operand reference.
       size_t DigitStart = Pos;
+      if (AsmString[DigitStart] == '{') {
+        OS << '{';
+        ++DigitStart;
+      }
       size_t DigitEnd = AsmString.find_first_not_of("0123456789", DigitStart);
       if (DigitEnd == std::string::npos)
         DigitEnd = AsmString.size();

clang/test/CodeGen/mozilla-ms-inline-asm.c

@@ -27,7 +27,7 @@ void invoke(void* that, unsigned methodIndex,
 // CHECK-SAME: sub esp,eax
 // CHECK-SAME: mov ecx,esp
 // CHECK-SAME: push $0
-// CHECK-SAME: call dword ptr $2
+// CHECK-SAME: call dword ptr ${2:P}
 // CHECK-SAME: {{.*}}__MSASMLABEL_.${:uid}__noparams:
 // CHECK-SAME: mov ecx,$3
 // CHECK-SAME: push ecx

clang/test/CodeGen/ms-inline-asm.c

@@ -306,7 +306,7 @@ void t24_helper(void) {}
 void t24() {
   __asm call t24_helper
 // CHECK: t24
-// CHECK: call void asm sideeffect inteldialect "call dword ptr $0", "*m,~{dirflag},~{fpsr},~{flags}"(void ()* @t24_helper)
+// CHECK: call void asm sideeffect inteldialect "call dword ptr ${0:P}", "*m,~{dirflag},~{fpsr},~{flags}"(void ()* @t24_helper)
 }
 
 void t25() {
@@ -681,7 +681,7 @@ void dot_operator(){
 void call_clobber() {
   __asm call t41
   // CHECK-LABEL: define void @call_clobber
-  // CHECK: call void asm sideeffect inteldialect "call dword ptr $0", "*m,~{dirflag},~{fpsr},~{flags}"(void (i16)* @t41)
+  // CHECK: call void asm sideeffect inteldialect "call dword ptr ${0:P}", "*m,~{dirflag},~{fpsr},~{flags}"(void (i16)* @t41)
 }
 
 void xgetbv() {

clang/test/CodeGen/ms-inline-asm.cpp

@@ -109,7 +109,7 @@ void test5() {
   __asm mov x, eax
   // CHECK: call void asm sideeffect inteldialect
   // CHECK-SAME: push $0
-  // CHECK-SAME: call dword ptr $2
+  // CHECK-SAME: call dword ptr ${2:P}
   // CHECK-SAME: mov $1, eax
   // CHECK-SAME: "=*m,=*m,*m,~{esp},~{dirflag},~{fpsr},~{flags}"(i32* %y, i32* %x, i32 (float)* @_ZN2T5IiE6createIfEEiT_)
 }

llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h

@@ -71,6 +71,10 @@ class MCParsedAsmOperand {
   /// variable/label?   Only valid when parsing MS-style inline assembly.
   virtual bool needAddressOf() const { return false; }
 
+  /// isCallOperand - Is this an operand of an inline-assembly call instruction?
+  /// Only valid when parsing MS-style inline assembly.
+  virtual bool isCallOperand() const { return false; }
+
   /// isOffsetOf - Do we need to emit code to get the offset of the variable,
   /// rather then the value of the variable?   Only valid when parsing MS-style
   /// inline assembly.

llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h

@@ -35,6 +35,7 @@ enum AsmRewriteKind {
   AOK_Align,          // Rewrite align as .align.
   AOK_EVEN,           // Rewrite even as .even.
   AOK_Emit,           // Rewrite _emit as .byte.
+  AOK_CallInput,      // Rewrite in terms of ${N:P}.
   AOK_Input,          // Rewrite in terms of $N.
   AOK_Output,         // Rewrite in terms of $N.
   AOK_SizeDirective,  // Add a sizing directive (e.g., dword ptr).
@@ -49,6 +50,7 @@ const char AsmRewritePrecedence [] = {
   2, // AOK_EVEN
   2, // AOK_Emit
   3, // AOK_Input
+  3, // AOK_CallInput
   3, // AOK_Output
   5, // AOK_SizeDirective
   1, // AOK_Label

llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

@@ -207,11 +207,17 @@ static void EmitMSInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
       }
       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 (LastEmitted[0] == '{' && LastEmitted[1] == ':') {
-        LastEmitted += 2;
+      if (HasCurlyBraces && LastEmitted[0] == ':') {
+        ++LastEmitted;
         const char *StrStart = LastEmitted;
         const char *StrEnd = strchr(StrStart, '}');
         if (!StrEnd)
@@ -238,6 +244,27 @@ static void EmitMSInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
         report_fatal_error("Invalid $ operand number in inline asm string: '" +
                            Twine(AsmStr) + "'");
 
+      char Modifier[2] = { 0, 0 };
+
+      if (HasCurlyBraces) {
+        // If we have curly braces, check for a modifier character.  This
+        // supports syntax like ${0:u}, which correspond to "%u0" in GCC asm.
+        if (*LastEmitted == ':') {
+          ++LastEmitted;    // Consume ':' character.
+          if (*LastEmitted == 0)
+            report_fatal_error("Bad ${:} expression in inline asm string: '" +
+                               Twine(AsmStr) + "'");
+
+          Modifier[0] = *LastEmitted;
+          ++LastEmitted;    // Consume modifier character.
+        }
+
+        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!
       unsigned OpNo = InlineAsm::MIOp_FirstOperand;
@@ -262,9 +289,11 @@ static void EmitMSInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
         ++OpNo;  // Skip over the ID number.
 
         if (InlineAsm::isMemKind(OpFlags)) {
-          Error = AP->PrintAsmMemoryOperand(MI, OpNo, /*Modifier*/ nullptr, OS);
+          Error = AP->PrintAsmMemoryOperand(
+              MI, OpNo, Modifier[0] ? Modifier : nullptr, OS);
         } else {
-          Error = AP->PrintAsmOperand(MI, OpNo, /*Modifier*/ nullptr, OS);
+          Error = AP->PrintAsmOperand(MI, OpNo,
+                                      Modifier[0] ? Modifier : nullptr, OS);
         }
       }
       if (Error) {

llvm/lib/MC/MCParser/AsmParser.cpp

@@ -5728,7 +5728,10 @@ bool AsmParser::parseMSInlineAsm(
         InputDecls.push_back(OpDecl);
         InputDeclsAddressOf.push_back(Operand.needAddressOf());
         InputConstraints.push_back(Operand.getConstraint().str());
-        AsmStrRewrites.emplace_back(AOK_Input, Start, SymName.size());
+        if (Operand.isCallOperand())
+          AsmStrRewrites.emplace_back(AOK_CallInput, Start, SymName.size());
+        else
+          AsmStrRewrites.emplace_back(AOK_Input, Start, SymName.size());
       }
     }
 
@@ -5818,6 +5821,9 @@ bool AsmParser::parseMSInlineAsm(
     case AOK_Input:
       OS << '$' << InputIdx++;
       break;
+    case AOK_CallInput:
+      OS << "${" << InputIdx++ << ":P}";
+      break;
     case AOK_Output:
       OS << '$' << OutputIdx++;
       break;

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

@@ -2855,6 +2855,15 @@ bool X86AsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
     }
   }
 
+  // Mark the operands of a call instruction.  These need to be handled
+  // differently when referenced in MS-style inline assembly.
+  if (Name.startswith("call") || Name.startswith("lcall")) {
+    for (size_t i = 1; i < Operands.size(); ++i) {
+      X86Operand &Op = static_cast<X86Operand &>(*Operands[i]);
+      Op.setCallOperand(true);
+    }
+  }
+
   if (Flags)
     Operands.push_back(X86Operand::CreatePrefix(Flags, NameLoc, NameLoc));
   return false;

llvm/lib/Target/X86/AsmParser/X86Operand.h

@@ -36,6 +36,7 @@ struct X86Operand final : public MCParsedAsmOperand {
   StringRef SymName;
   void *OpDecl;
   bool AddressOf;
+  bool CallOperand;
 
   struct TokOp {
     const char *Data;
@@ -77,7 +78,7 @@ struct X86Operand final : public MCParsedAsmOperand {
   };
 
   X86Operand(KindTy K, SMLoc Start, SMLoc End)
-      : Kind(K), StartLoc(Start), EndLoc(End) {}
+      : Kind(K), StartLoc(Start), EndLoc(End), CallOperand(false) {}
 
   StringRef getSymName() override { return SymName; }
   void *getOpDecl() override { return OpDecl; }
@@ -277,6 +278,9 @@ struct X86Operand final : public MCParsedAsmOperand {
     return AddressOf;
   }
 
+  bool isCallOperand() const override { return CallOperand; }
+  void setCallOperand(bool IsCallOperand) { CallOperand = IsCallOperand; }
+
   bool isMem() const override { return Kind == Memory; }
   bool isMemUnsized() const {
     return Kind == Memory && Mem.Size == 0;

llvm/lib/Target/X86/X86AsmPrinter.cpp

@@ -336,14 +336,22 @@ void X86AsmPrinter::PrintMemReference(const MachineInstr *MI, unsigned OpNo,
   PrintLeaMemReference(MI, OpNo, O, Modifier);
 }
 
+
 void X86AsmPrinter::PrintIntelMemReference(const MachineInstr *MI,
-                                           unsigned OpNo, raw_ostream &O) {
+                                           unsigned OpNo, raw_ostream &O,
+                                           const char *Modifier) {
   const MachineOperand &BaseReg = MI->getOperand(OpNo + X86::AddrBaseReg);
   unsigned ScaleVal = MI->getOperand(OpNo + X86::AddrScaleAmt).getImm();
   const MachineOperand &IndexReg = MI->getOperand(OpNo + X86::AddrIndexReg);
   const MachineOperand &DispSpec = MI->getOperand(OpNo + X86::AddrDisp);
   const MachineOperand &SegReg = MI->getOperand(OpNo + X86::AddrSegmentReg);
 
+  // If we really don't want to print out (rip), don't.
+  bool HasBaseReg = BaseReg.getReg() != 0;
+  if (HasBaseReg && Modifier && !strcmp(Modifier, "no-rip") &&
+      BaseReg.getReg() == X86::RIP)
+    HasBaseReg = false;
+
   // If this has a segment register, print it.
   if (SegReg.getReg()) {
     PrintOperand(MI, OpNo + X86::AddrSegmentReg, O);
@@ -353,7 +361,7 @@ void X86AsmPrinter::PrintIntelMemReference(const MachineInstr *MI,
   O << '[';
 
   bool NeedPlus = false;
-  if (BaseReg.getReg()) {
+  if (HasBaseReg) {
     PrintOperand(MI, OpNo + X86::AddrBaseReg, O);
     NeedPlus = true;
   }
@@ -371,7 +379,7 @@ void X86AsmPrinter::PrintIntelMemReference(const MachineInstr *MI,
     PrintOperand(MI, OpNo + X86::AddrDisp, O);
   } else {
     int64_t DispVal = DispSpec.getImm();
-    if (DispVal || (!IndexReg.getReg() && !BaseReg.getReg())) {
+    if (DispVal || (!IndexReg.getReg() && !HasBaseReg)) {
       if (NeedPlus) {
         if (DispVal > 0)
           O << " + ";
@@ -524,11 +532,6 @@ bool X86AsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
 bool X86AsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo,
                                           const char *ExtraCode,
                                           raw_ostream &O) {
-  if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
-    PrintIntelMemReference(MI, OpNo, O);
-    return false;
-  }
-
   if (ExtraCode && ExtraCode[0]) {
     if (ExtraCode[1] != 0) return true; // Unknown modifier.
 
@@ -542,14 +545,26 @@ bool X86AsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo,
       // These only apply to registers, ignore on mem.
       break;
     case 'H':
-      PrintMemReference(MI, OpNo, O, "H");
+      if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
+        return true;  // Unsupported modifier in Intel inline assembly.
+      } else {
+        PrintMemReference(MI, OpNo, O, "H");
+      }
       return false;
     case 'P': // Don't print @PLT, but do print as memory.
-      PrintMemReference(MI, OpNo, O, "no-rip");
+      if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
+        PrintIntelMemReference(MI, OpNo, O, "no-rip");
+      } else {
+        PrintMemReference(MI, OpNo, O, "no-rip");
+      }
       return false;
     }
   }
-  PrintMemReference(MI, OpNo, O, nullptr);
+  if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
+    PrintIntelMemReference(MI, OpNo, O, nullptr);
+  } else {
+    PrintMemReference(MI, OpNo, O, nullptr);
+  }
   return false;
 }
 

llvm/lib/Target/X86/X86AsmPrinter.h

@@ -112,7 +112,7 @@ class LLVM_LIBRARY_VISIBILITY X86AsmPrinter : public AsmPrinter {
   void PrintMemReference(const MachineInstr *MI, unsigned OpNo, raw_ostream &O,
                          const char *Modifier);
   void PrintIntelMemReference(const MachineInstr *MI, unsigned OpNo,
-                              raw_ostream &O);
+                              raw_ostream &O, const char *Modifier);
 
 public:
   X86AsmPrinter(TargetMachine &TM, std::unique_ptr<MCStreamer> Streamer);

llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll

@@ -0,0 +1,18 @@
+; RUN: llc < %s -mtriple=i686-- | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-- | FileCheck %s
+
+define void @func() {
+entry:
+  ret void
+}
+
+define void @main() {
+entry:
+  call void asm sideeffect inteldialect "call ${0:P}", "*m,~{dirflag},~{fpsr},~{flags}"(void ()* @func)
+  ret void
+; CHECK-LABEL: main:
+; CHECK: {{## InlineAsm Start|#APP}}
+; CHECK: {{call(l|q) func$}}
+; CHECK: {{## InlineAsm End|#NO_APP}}
+; CHECK: ret{{l|q}}
+}