[X86][MS-InlineAsm] Make the constraint *m to be simple place holder

D113096 solved the "undefined reference to xxx" issue by adding constraint *m for the global var. But it has strong side effect due to the symbol in the assembly being replaced with constraint variable. This leads to some lowering fails. https://godbolt.org/z/h3nWoerPe This patch fix the problem by use the constraint *m as place holder rather than real constraint. It has negligible effect for the existing code generation. Reviewed By: skan Differential Revision: https://reviews.llvm.org/D115225
llvm · Dec 10, 2021 · d7c07f6 · d7c07f6
1 parent afa3c14
commit d7c07f6
Show file tree

Hide file tree

Showing 9 changed files with 32 additions and 17 deletions.
diff --git a/clang/test/CodeGen/X86/ms_fmul.c b/clang/test/CodeGen/X86/ms_fmul.c
@@ -18,4 +18,4 @@ void __attribute__ ((naked)) foo(void)
 }}
 
 // CHECK-LABEL: foo
-// CHECK: call void asm sideeffect inteldialect "fmul qword ptr $0[edx + $$240]\0A\09ret"
+// CHECK: call void asm sideeffect inteldialect "fmul qword ptr static_const_table[edx + $$240]\0A\09ret"
diff --git a/clang/test/CodeGen/ms-inline-asm-functions.c b/clang/test/CodeGen/ms-inline-asm-functions.c
@@ -39,7 +39,7 @@ int bar() {
 int baz() {
   // CHECK-LABEL: _baz:
   __asm mov eax, k;
-  // CHECK: movl    _k, %eax
+  // CHECK: movl    k, %eax
   __asm mov eax, kptr;
   // CHECK: movl    _kptr, %eax
 }

diff --git a/clang/test/CodeGen/ms-inline-asm-static-variable.c b/clang/test/CodeGen/ms-inline-asm-static-variable.c
@@ -5,6 +5,6 @@
 static int arr[10];
 void t1() {
   // CHECK: @arr = internal global [10 x i32]
-  // CHECK: call void asm sideeffect inteldialect "mov dword ptr $0[edx * $$4],edx", "=*m,{{.*}}([10 x i32]* @arr)
+  // CHECK: call void asm sideeffect inteldialect "mov dword ptr arr[edx * $$4],edx", "=*m,{{.*}}([10 x i32]* @arr)
   __asm mov  dword ptr arr[edx*4],edx
 }
diff --git a/clang/test/CodeGen/ms-inline-asm-variables.c b/clang/test/CodeGen/ms-inline-asm-variables.c
@@ -3,19 +3,19 @@
 
 int gVar;
 void t1() {
-  // CHECK: add eax, dword ptr ${{[0-9]}}[eax]
+  // CHECK: add eax, dword ptr gVar[eax]
   __asm add eax, dword ptr gVar[eax]
-  // CHECK: add dword ptr ${{[0-9]}}[eax], eax
+  // CHECK: add dword ptr gVar[eax], eax
   __asm add dword ptr [eax+gVar], eax
-  // CHECK: add ebx, dword ptr ${{[0-9]}}[ebx + $$270]
+  // CHECK: add ebx, dword ptr gVar[ebx + $$270]
   __asm add ebx, dword ptr gVar[271 - 82 + 81 + ebx]
-  // CHECK: add dword ptr ${{[0-9]}}[ebx + $$828], ebx
+  // CHECK: add dword ptr gVar[ebx + $$828], ebx
   __asm add dword ptr [ebx + gVar + 828], ebx
-  // CHECK: add ecx, dword ptr ${{[0-9]}}[ecx + ecx * $$4 + $$4590]
+  // CHECK: add ecx, dword ptr gVar[ecx + ecx * $$4 + $$4590]
   __asm add ecx, dword ptr gVar[4590 + ecx + ecx*4]
-  // CHECK: add dword ptr ${{[0-9]}}[ecx + ecx * $$8 + $$73], ecx
+  // CHECK: add dword ptr gVar[ecx + ecx * $$8 + $$73], ecx
   __asm add dword ptr [gVar + ecx + 45 + 23 - 53 + 60 - 2 + ecx*8], ecx
-  // CHECK: add ${{[0-9]}}[ecx + ebx + $$7], eax
+  // CHECK: add gVar[ecx + ebx + $$7], eax
   __asm add 1 + 1 + 2 + 3[gVar + ecx + ebx], eax
 }
 

diff --git a/llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h b/llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
@@ -10,6 +10,7 @@
 #define LLVM_MC_MCPARSER_MCPARSEDASMOPERAND_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/MC/MCInstrDesc.h"
 #include "llvm/Support/SMLoc.h"
 #include <string>
 
@@ -76,6 +77,10 @@ class MCParsedAsmOperand {
   /// assembly.
   virtual bool isOffsetOfLocal() const { return false; }
 
+  /// isMemPlaceholder - Do we need to ignore the constraint, rather than emit
+  /// code? Only valid when parsing MS-style inline assembly.
+  virtual bool isMemPlaceholder(const MCInstrDesc &Desc) const { return false; }
+
   /// getOffsetOfLoc - Get the location of the offset operator.
   virtual SMLoc getOffsetOfLoc() const { return SMLoc(); }
 

diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -6012,20 +6012,21 @@ bool AsmParser::parseMSInlineAsm(
 
       bool isOutput = (i == 1) && Desc.mayStore();
       SMLoc Start = SMLoc::getFromPointer(SymName.data());
+      int64_t Size = Operand.isMemPlaceholder(Desc) ? 0 : SymName.size();
       if (isOutput) {
         ++InputIdx;
         OutputDecls.push_back(OpDecl);
         OutputDeclsAddressOf.push_back(Operand.needAddressOf());
         OutputConstraints.push_back(("=" + Constraint).str());
-        AsmStrRewrites.emplace_back(AOK_Output, Start, SymName.size());
+        AsmStrRewrites.emplace_back(AOK_Output, Start, Size);
       } else {
         InputDecls.push_back(OpDecl);
         InputDeclsAddressOf.push_back(Operand.needAddressOf());
         InputConstraints.push_back(Constraint.str());
         if (Desc.OpInfo[i - 1].isBranchTarget())
           AsmStrRewrites.emplace_back(AOK_CallInput, Start, SymName.size());
         else
-          AsmStrRewrites.emplace_back(AOK_Input, Start, SymName.size());
+          AsmStrRewrites.emplace_back(AOK_Input, Start, Size);
       }
     }
 
@@ -6140,13 +6141,17 @@ bool AsmParser::parseMSInlineAsm(
       OS << Ctx.getAsmInfo()->getPrivateLabelPrefix() << AR.Label;
       break;
     case AOK_Input:
-      OS << '$' << InputIdx++;
+      if (AR.Len)
+        OS << '$' << InputIdx;
+      ++InputIdx;
       break;
     case AOK_CallInput:
       OS << "${" << InputIdx++ << ":P}";
       break;
     case AOK_Output:
-      OS << '$' << OutputIdx++;
+      if (AR.Len)
+        OS << '$' << OutputIdx;
+      ++OutputIdx;
       break;
     case AOK_SizeDirective:
       switch (AR.Val) {

diff --git a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -2551,8 +2551,6 @@ bool X86AsmParser::ParseIntelOperand(OperandVector &Operands) {
   StringRef ErrMsg;
   unsigned BaseReg = SM.getBaseReg();
   unsigned IndexReg = SM.getIndexReg();
-  if (IndexReg && BaseReg == X86::RIP)
-    BaseReg = 0;
   unsigned Scale = SM.getScale();
   if (!PtrInOperand)
     Size = SM.getElementSize() << 3;

diff --git a/llvm/lib/Target/X86/AsmParser/X86Operand.h b/llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -285,6 +285,13 @@ struct X86Operand final : public MCParsedAsmOperand {
 
   bool isOffsetOfLocal() const override { return isImm() && Imm.LocalRef; }
 
+  bool isMemPlaceholder(const MCInstrDesc &Desc) const override {
+    // Add more restrictions to avoid the use of global symbols. This helps
+    // with reducing the code size.
+    return !Desc.isRematerializable() && !Desc.isCall() && isMem() &&
+           !Mem.BaseReg && !Mem.IndexReg;
+  }
+
   bool needAddressOf() const override { return AddressOf; }
 
   bool isMem() const override { return Kind == Memory; }

diff --git a/llvm/test/CodeGen/X86/ms-inline-asm-array.ll b/llvm/test/CodeGen/X86/ms-inline-asm-array.ll
@@ -5,7 +5,7 @@
 ; CHECK: movl    %edx, arr(,%rdx,4)
 define dso_local i32 @main() #0 {
 entry:
-  call void asm sideeffect inteldialect "mov dword ptr $0[rdx * $$4],edx", "=*m,~{dirflag},~{fpsr},~{flags}"([10 x i32]* @arr) #1, !srcloc !4
+  call void asm sideeffect inteldialect "mov dword ptr arr[rdx * $$4],edx", "=*m,~{dirflag},~{fpsr},~{flags}"([10 x i32]* @arr) #1, !srcloc !4
   ret i32 0
 }