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

[pengfei]

Summary

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.

[skan]

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

42

This change is strange here. Others looks good to me.

[pengfei]

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

42

Yeah, but we can't roll back all the constraint added D113096 unless we add a separate flag to record it.
In fact, using the symbol won't result in any functionality issue. And in this case, I checked the built binary is the same as the one before this patch.

[skan]

LGTM

[pengfei]

Thanks for the review!

clang/test/CodeGen/X86/ms_fmul.c

 
 void __attribute__ ((naked)) foo(void)
 {__asm{
     fmul qword ptr [static_const_table + 0x00f0 +edx]
     ret
 }}
 
 // 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"

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

int bar() {
   // CHECK-LABEL: _bar:
   __asm jmp k;
   // CHECK:     jmp     _k
 }
 
 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
 }
 
 // Test that this asm blob doesn't require more registers than available.  This
 // has to be an LLVM code generation test.
 
 void __declspec(naked) naked() {

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

 // REQUIRES: x86-registered-target
 // Check the constraint "*m" of operand arr and the definition of arr is not removed by FE
 // RUN: %clang_cc1 %s -fasm-blocks -triple i386-apple-darwin10 -emit-llvm -o - | FileCheck %s
 
 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
 }

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

 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 %s -fasm-blocks -triple i386-apple-darwin10 -emit-llvm -o - | FileCheck %s
 
 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
 }
 
 void t2() {
   int lVar;
   // CHECK: mov eax, dword ptr ${{[0-9]}}[eax]
   __asm mov eax, dword ptr lVar[eax]
   // CHECK: mov dword ptr ${{[0-9]}}[eax], eax
   __asm mov dword ptr [eax+lVar], eax
   // CHECK: mov ebx, dword ptr ${{[0-9]}}[ebx + $$270]
   __asm mov ebx, dword ptr lVar[271 - 82 + 81 + ebx]
   // CHECK: mov dword ptr ${{[0-9]}}[ebx + $$828], ebx
   __asm mov dword ptr [ebx + lVar + 828], ebx
   // CHECK: mov ${{[0-9]}}[ebx + $$47], eax
   __asm mov 5 + 8 + 13 + 21[lVar + ebx], eax
 }

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

 //===- llvm/MC/MCParsedAsmOperand.h - Asm Parser Operand --------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_MC_MCPARSER_MCPARSEDASMOPERAND_H
 #define LLVM_MC_MCPARSER_MCPARSEDASMOPERAND_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/MC/MCInstrDesc.h"
 #include "llvm/Support/SMLoc.h"
 #include <string>
 
 namespace llvm {
 
 class raw_ostream;
 
 /// MCParsedAsmOperand - This abstract class represents a source-level assembly
public:
   /// variable/label?   Only valid when parsing MS-style inline assembly.
   virtual bool needAddressOf() const { return false; }
 
   /// isOffsetOfLocal - Do we need to emit code to get the offset of the local
   /// variable, rather than its value?   Only valid when parsing MS-style inline
   /// 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(); }
 
   /// print - Print a debug representation of the operand to the given stream.
   virtual void print(raw_ostream &OS) const = 0;
 
   /// dump - Print to the debug stream.
   virtual void dump() const;

llvm/lib/MC/MCParser/AsmParser.cpp

for (unsigned i = 1, e = Info.ParsedOperands.size(); i != e; ++i) {
         if (Operand.isOffsetOfLocal())
           Constraint = "r";
         else
           Constraint = "i";
       }
 
       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);
       }
     }
 
     // Consider implicit defs to be clobbers.  Think of cpuid and push.
     ArrayRef<MCPhysReg> ImpDefs(Desc.getImplicitDefs(),
                                 Desc.getNumImplicitDefs());
     llvm::append_range(ClobberRegs, ImpDefs);
   }
case AOK_IntelExpr:
         OS << (AR.IntelExp.emitImm() ? "$$" : " + $$") << AR.IntelExp.Imm;
       if (AR.IntelExp.NeedBracs)
         OS << "]";
       break;
     case AOK_Label:
       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) {
       default: break;
       case 8:  OS << "byte ptr "; break;
       case 16: OS << "word ptr "; break;
       case 32: OS << "dword ptr "; break;
       case 64: OS << "qword ptr "; break;

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

if (!SM.isMemExpr() && !RegNo) {
 
     Operands.push_back(X86Operand::CreateImm(Disp, Start, End));
     return false;
   }
 
   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;
 
   if (Scale == 0 && BaseReg != X86::ESP && BaseReg != X86::RSP &&
       (IndexReg == X86::ESP || IndexReg == X86::RSP))
     std::swap(BaseReg, IndexReg);
 

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

bool isImmUnsignedi8() const {
     // handle it.
     const MCConstantExpr *CE = dyn_cast<MCConstantExpr>(getImm());
     if (!CE) return true;
     return isImmUnsignedi8Value(CE->getValue());
   }
 
   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; }
   bool isMemUnsized() const {
     return Kind == Memory && Mem.Size == 0;
   }
   bool isMem8() const {
     return Kind == Memory && (!Mem.Size || Mem.Size == 8);

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

 ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
 
 @arr = internal global [10 x i32] zeroinitializer, align 16
 
 ; 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
 }
 
 attributes #0 = { noinline nounwind optnone uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
 attributes #1 = { nounwind }
 
 !llvm.module.flags = !{!0, !1, !2}
 !llvm.ident = !{!3}
 
 !0 = !{i32 1, !"wchar_size", i32 4}
 !1 = !{i32 7, !"uwtable", i32 1}
 !2 = !{i32 7, !"frame-pointer", i32 2}
 !3 = !{!"clang"}
 !4 = !{i64 63}