ms-inline-asm: Add facilities for rewriting labels in X86AsmParser

[• ehsan]

Summary

This will be used in order to make it possible to rewrite asm labels
to use internal names, so that we can scope the label names to
functions as opposed to LLVM inline asm blocks.

ms-inline-asm: Add a sema callback for looking up label names

The implementation of the callback in clang's Sema will return an
internal name for labels.

[rnk]

include/llvm/MC/MCParser/MCAsmParser.h

57

It's generally nicer to avoid having API breakage between LLVM in clang if it can be avoided. In this case, I'd give this a no-op implementation that clang overrides instead of a pure virtual method which will break the clang build for a few revisions when this lands.

lib/MC/MCParser/AsmParser.cpp

4620

This should really be making an local label that doesn't make it into the object file. MC should know what the local label prefix is. It's usually 'L', I think.

If you add the prefix here, then maybe this should be AOK_Label or something.

lib/Target/X86/AsmParser/X86AsmParser.cpp

1305

What does MSVC do in this case? I assume it uses the opposite logic for this code:

void foo();
void __declspec(naked) bar() {
  __asm {
  foo:
    jmp foo ; probably infinite loops instead of tail calling bar.
  }
}

I guess this is the hard case:

void foo();
void __declspec(naked) bar() {
  __asm {
    test ecx, ecx
    xor eax, eax
    jz foo
    mov eax 1
foo:
   ret
}

[• ehsan]

I'll address the rest of the comments in the next iteration of this and the LLVM side patch.

lib/MC/MCParser/AsmParser.cpp

4620

What do you mean by a local label? I don't think that something that doesn't make it into the object file is what we want here. For example, MSVC creates a relocation to $label$1 in this test case:

void f() {

__asm {
  label:
  mov eax, label
}

}

lib/Target/X86/AsmParser/X86AsmParser.cpp

1305

The first case is turned into:

_bar:

0:       e9 00 00 00 00                                  jmp     0
                 00000001:  IMAGE_REL_I386_REL32 _foo

The second case is turned into:

_bar:

0:       85 c9                                           testl   %ecx, %ecx
2:       33 c0                                           xorl    %eax, %eax
4:       0f 84 00 00 00 00                               je      0
                 00000006:  IMAGE_REL_I386_REL32 _foo
a:       b8 01 00 00 00                                  movl    $1, %eax
f:       c3                                              retl

My patch compiles the first case into:

_bar:

       0:	b8 00 00 00 00                               	movl	$0, %eax
			       1: IMAGE_REL_I386_DIR32	_foo

MSASMLABEL_.0:

5:	ff e0                                        	jmpl	*%eax
7:	c3                                           	ret

And the second case into:

_bar:

       0:	85 c9                                        	testl	%ecx, %ecx
       2:	31 c0                                        	xorl	%eax, %eax
       4:	0f 84 00 00 00 00                            	je	0
			       6: IMAGE_REL_I386_REL32	foo
       a:	b8 01 00 00 00                               	movl	$1, %eax

MSASMLABEL_.0:

 f:	c3                                           	ret
10:	c3                                           	ret

Is that good (enough)?

[• ehsan]

Addressed the review comments.

[• ehsan]

Updated the patch to use the LabelDecl machinery on the clang side.

[• ehsan]

Can you please review this too? Thanks!

[rnk]

lgtm

lib/MC/MCParser/AsmParser.cpp

4620

Hm, true. I think MC will make a relocation for this as well even if you use the private prefix.

lib/Target/X86/AsmParser/X86AsmParser.cpp

1305

Huh, so they pick global functions before local labels, and so do we. Seems fine then.

[• ehsan]

Closed by commit rL218229 (authored by @ehsan).

include/llvm/MC/MCParser/MCAsmParser.h

 class SourceMgr;
 class Twine;
 
 class InlineAsmIdentifierInfo {
 public:
   void *OpDecl;
   bool IsVarDecl;
   unsigned Length, Size, Type;
+  StringRef InternalName;
 
   void clear() {
     OpDecl = nullptr;
     IsVarDecl = false;
     Length = 1;
     Size = 0;
     Type = 0;
+    InternalName = "";
   }
 };
 
 /// MCAsmParserSemaCallback - Generic Sema callback for assembly parser.
 class MCAsmParserSemaCallback {
 public:
   virtual ~MCAsmParserSemaCallback();
   virtual void *LookupInlineAsmIdentifier(StringRef &LineBuf,
                                           InlineAsmIdentifierInfo &Info,
                                           bool IsUnevaluatedContext) = 0;
+  virtual void LookupInlineAsmLabel(StringRef &Identifier, SourceMgr &SM, SMLoc Location) = 0;
 
   virtual bool LookupInlineAsmField(StringRef Base, StringRef Member,
                                     unsigned &Offset) = 0;
 };
 
 /// MCAsmParser - Generic assembler parser interface, for use by target specific
 /// assembly parsers.
 class MCAsmParser {

include/llvm/MC/MCTargetAsmParser.h

enum AsmRewriteKind {
   AOK_DotOperator,    // Rewrite a dot operator expression as an immediate.
                       // E.g., [eax].foo.bar -> [eax].8
   AOK_Emit,           // Rewrite _emit as .byte.
   AOK_Imm,            // Rewrite as $$N.
   AOK_ImmPrefix,      // Add $$ before a parsed Imm.
   AOK_Input,          // Rewrite in terms of $N.
   AOK_Output,         // Rewrite in terms of $N.
   AOK_SizeDirective,  // Add a sizing directive (e.g., dword ptr).
+  AOK_Identifier,     // Rewrite the identifier.
   AOK_Skip            // Skip emission (e.g., offset/type operators).
 };
 
 const char AsmRewritePrecedence [] = {
   0, // AOK_Delete
   1, // AOK_Align
   1, // AOK_DotOperator
   1, // AOK_Emit
   3, // AOK_Imm
   3, // AOK_ImmPrefix
   2, // AOK_Input
   2, // AOK_Output
   4, // AOK_SizeDirective
+  1, // AOK_Identifier
   1  // AOK_Skip
 };
 
 struct AsmRewrite {
   AsmRewriteKind Kind;
   SMLoc Loc;
   unsigned Len;
   unsigned Val;
+  StringRef Identifier;
 public:
   AsmRewrite(AsmRewriteKind kind, SMLoc loc, unsigned len = 0, unsigned val = 0)
     : Kind(kind), Loc(loc), Len(len), Val(val) {}
+  AsmRewrite(AsmRewriteKind kind, SMLoc loc, unsigned len, StringRef identifier)
+    : Kind(kind), Loc(loc), Len(len), Val(0), Identifier(identifier) {}
 };
 
 struct ParseInstructionInfo {
 
   SmallVectorImpl<AsmRewrite> *AsmRewrites;
 
   ParseInstructionInfo() : AsmRewrites(nullptr) {}
   ParseInstructionInfo(SmallVectorImpl<AsmRewrite> *rewrites)

lib/MC/MCParser/AsmParser.cpp

for (const AsmRewrite &AR : AsmStrRewrites) {
     default:
       break;
     case AOK_Imm:
       OS << "$$" << AR.Val;
       break;
     case AOK_ImmPrefix:
       OS << "$$";
       break;
+    case AOK_Identifier:
+      OS << AR.Identifier;
+      break;
     case AOK_Input:
       OS << '$' << InputIdx++;
       break;
     case AOK_Output:
       OS << '$' << OutputIdx++;
       break;
     case AOK_SizeDirective:
       switch (AR.Val) {

lib/Target/X86/AsmParser/X86AsmParser.cpp

std::unique_ptr<X86Operand> X86AsmParser::CreateMemForInlineAsm(
     unsigned SegReg, const MCExpr *Disp, unsigned BaseReg, unsigned IndexReg,
     unsigned Scale, SMLoc Start, SMLoc End, unsigned Size, StringRef Identifier,
     InlineAsmIdentifierInfo &Info) {
   // If this is not a VarDecl then assume it is a FuncDecl or some other label
   // reference.  We need an 'r' constraint here, so we need to create register
   // operand to ensure proper matching.  Just pick a GPR based on the size of
   // a pointer.
   if (isa<MCSymbolRefExpr>(Disp) && !Info.IsVarDecl) {
+    if (!Info.InternalName.empty()) {
+      // Push a rewrite for replacing the identifier name with the internal name.
+      InstInfo->AsmRewrites->push_back(AsmRewrite(AOK_Identifier, Start,
+                                                  End.getPointer() - Start.getPointer(),
+                                                  Info.InternalName));
+    }
     unsigned RegNo =
         is64BitMode() ? X86::RBX : (is32BitMode() ? X86::EBX : X86::BX);
     return X86Operand::CreateReg(RegNo, Start, End, /*AddressOf=*/true,
                                  SMLoc(), Identifier, Info.OpDecl);
   }
 
   // We either have a direct symbol reference, or an offset from a symbol.  The
   // parser always puts the symbol on the LHS, so look there for size

 bool X86AsmParser::ParseIntelIdentifier(const MCExpr *&Val,
                                         StringRef &Identifier,
                                         InlineAsmIdentifierInfo &Info,
                                         bool IsUnevaluatedOperand, SMLoc &End) {
   assert (isParsingInlineAsm() && "Expected to be parsing inline assembly.");
   Val = nullptr;
 
   StringRef LineBuf(Identifier.data());
+  void *Result =
     SemaCallback->LookupInlineAsmIdentifier(LineBuf, Info, IsUnevaluatedOperand);
 
   const AsmToken &Tok = Parser.getTok();
+  SMLoc Loc = Tok.getLoc();
 
   // Advance the token stream until the end of the current token is
   // after the end of what the frontend claimed.
   const char *EndPtr = Tok.getLoc().getPointer() + LineBuf.size();
   while (true) {
     End = Tok.getEndLoc();
     getLexer().Lex();
 
     assert(End.getPointer() <= EndPtr && "frontend claimed part of a token?");
     if (End.getPointer() == EndPtr) break;
   }
+  Identifier = LineBuf;
+
+  // If the identifier lookup was unsuccessful, assume that we are dealing with
+  // a label.
+  if (!Result) {
+    SemaCallback->LookupInlineAsmLabel(Identifier, getSourceManager(), Loc);
+    if (Identifier != LineBuf) {
+      Info.InternalName = Identifier;
+    }
+  }
 
   // Create the symbol reference.
-  Identifier = LineBuf;
   MCSymbol *Sym = getContext().GetOrCreateSymbol(Identifier);
   MCSymbolRefExpr::VariantKind Variant = MCSymbolRefExpr::VK_None;
   Val = MCSymbolRefExpr::Create(Sym, Variant, getParser().getContext());
   return false;
 }
 
 /// \brief Parse intel style segment override.
 std::unique_ptr<X86Operand>