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 ↗

(On Diff #11658)

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 ↗

(On Diff #11658)

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 ↗

(On Diff #11658)

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 ↗

(On Diff #11658)

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 ↗

(On Diff #11658)

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 ↗

(On Diff #11658)

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 ↗

(On Diff #11658)

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

[• ehsan]

Closed by commit rL218229 (authored by @ehsan).

llvm/trunk/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 StringRef LookupInlineAsmLabel(StringRef Identifier, SourceMgr &SM,
+                                         SMLoc Location, bool Create)
+  { return nullptr; }
 
   virtual bool LookupInlineAsmField(StringRef Base, StringRef Member,
                                     unsigned &Offset) = 0;
 };
 
 /// MCAsmParser - Generic assembler parser interface, for use by target specific
 /// assembly parsers.
 class MCAsmParser {

llvm/trunk/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_Label,          // Rewrite local labels.
   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_Label
   1  // AOK_Skip
 };
 
 struct AsmRewrite {
   AsmRewriteKind Kind;
   SMLoc Loc;
   unsigned Len;
   unsigned Val;
+  StringRef Label;
 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 label)
+    : Kind(kind), Loc(loc), Len(len), Val(0), Label(label) {}
 };
 
 struct ParseInstructionInfo {
 
   SmallVectorImpl<AsmRewrite> *AsmRewrites;
 
   ParseInstructionInfo() : AsmRewrites(nullptr) {}
   ParseInstructionInfo(SmallVectorImpl<AsmRewrite> *rewrites)

llvm/trunk/lib/MC/MCParser/AsmParser.cpp

public:
   bool parseIdentifier(StringRef &Res) override;
   void eatToEndOfStatement() override;
 
   void checkForValidSection() override;
   /// }
 
 private:
 
-  bool parseStatement(ParseStatementInfo &Info);
+  bool parseStatement(ParseStatementInfo &Info,
+                      MCAsmParserSemaCallback *SI);
   void eatToEndOfLine();
   bool parseCppHashLineFilenameComment(const SMLoc &L);
 
   void checkForBadMacro(SMLoc DirectiveLoc, StringRef Name, StringRef Body,
                         ArrayRef<MCAsmMacroParameter> Parameters);
   bool expandMacro(raw_svector_ostream &OS, StringRef Body,
                    ArrayRef<MCAsmMacroParameter> Parameters,
                    ArrayRef<MCAsmMacroArgument> A,
if (getContext().getGenDwarfForAssembly()) {
     InsertResult.first->second.first = SectionStartSym;
     getContext().setGenDwarfFileNumber(getStreamer().EmitDwarfFileDirective(
         0, StringRef(), getContext().getMainFileName()));
   }
 
   // While we have input, parse each statement.
   while (Lexer.isNot(AsmToken::Eof)) {
     ParseStatementInfo Info;
-    if (!parseStatement(Info))
+    if (!parseStatement(Info, nullptr))
       continue;
 
     // We had an error, validate that one was emitted and recover by skipping to
     // the next line.
     assert(HadError && "Parse statement returned an error, but none emitted!");
     eatToEndOfStatement();
   }
 
while (1) {
     Res = MCBinaryExpr::Create(Kind, Res, RHS, getContext());
   }
 }
 
 /// ParseStatement:
 ///   ::= EndOfStatement
 ///   ::= Label* Directive ...Operands... EndOfStatement
 ///   ::= Label* Identifier OperandList* EndOfStatement
-bool AsmParser::parseStatement(ParseStatementInfo &Info) {
+bool AsmParser::parseStatement(ParseStatementInfo &Info,
+                               MCAsmParserSemaCallback *SI) {
   if (Lexer.is(AsmToken::EndOfStatement)) {
     Out.AddBlankLine();
     Lex();
     return false;
   }
 
   // Statements always start with an identifier or are a full line comment.
   AsmToken ID = getTok();
if (IDVal == ".")
       return Error(IDLoc, "invalid use of pseudo-symbol '.' as a label");
 
     // Diagnose attempt to use a variable as a label.
     //
     // FIXME: Diagnostics. Note the location of the definition as a label.
     // FIXME: This doesn't diagnose assignment to a symbol which has been
     // implicitly marked as external.
     MCSymbol *Sym;
-    if (LocalLabelVal == -1)
+    if (LocalLabelVal == -1) {
+      if (ParsingInlineAsm && SI) {
+        StringRef RewrittenLabel = SI->LookupInlineAsmLabel(IDVal, getSourceManager(), IDLoc, true);
+        assert(RewrittenLabel.size() && "We should have an internal name here.");
+        Info.AsmRewrites->push_back(AsmRewrite(AOK_Label, IDLoc,
+                                               IDVal.size(), RewrittenLabel));
+        IDVal = RewrittenLabel;
+      }
       Sym = getContext().GetOrCreateSymbol(IDVal);
-    else
+    } else
       Sym = Ctx.CreateDirectionalLocalSymbol(LocalLabelVal);
     if (!Sym->isUndefined() || Sym->isVariable())
       return Error(IDLoc, "invalid symbol redefinition");
 
     // Emit the label.
     if (!ParsingInlineAsm)
       Out.EmitLabel(Sym);
 
bool AsmParser::parseMSInlineAsm(
   // Prime the lexer.
   Lex();
 
   // While we have input, parse each statement.
   unsigned InputIdx = 0;
   unsigned OutputIdx = 0;
   while (getLexer().isNot(AsmToken::Eof)) {
     ParseStatementInfo Info(&AsmStrRewrites);
-    if (parseStatement(Info))
+    if (parseStatement(Info, &SI))
       return true;
 
     if (Info.ParseError)
       return true;
 
     if (Info.Opcode == ~0U)
       continue;
 
for (const AsmRewrite &AR : AsmStrRewrites) {
     default:
       break;
     case AOK_Imm:
       OS << "$$" << AR.Val;
       break;
     case AOK_ImmPrefix:
       OS << "$$";
       break;
+    case AOK_Label:
+      OS << Ctx.getAsmInfo()->getPrivateGlobalPrefix() << AR.Label;
+      break;
     case AOK_Input:
       OS << '$' << InputIdx++;
       break;
     case AOK_Output:
       OS << '$' << OutputIdx++;
       break;
     case AOK_SizeDirective:
       switch (AR.Val) {

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

bool IsSymRef =
       isa<MCSymbolRefExpr>(BinOp ? BinOp->getLHS() : Disp);
   if (IsSymRef) {
     if (!Size) {
       Size = Info.Type * 8; // Size is in terms of bits in this context.
       if (Size)
         InstInfo->AsmRewrites->push_back(AsmRewrite(AOK_SizeDirective, Start,
                                                     /*Len=*/0, Size));
     }
+    if (!Info.InternalName.empty()) {
+      // Push a rewrite for replacing the identifier name with the internal name.
+      InstInfo->AsmRewrites->push_back(AsmRewrite(AOK_Label, Start,
+                                                  End.getPointer() - Start.getPointer(),
+                                                  Info.InternalName));
+    }
   }
 
   // When parsing inline assembly we set the base register to a non-zero value
   // if we don't know the actual value at this time.  This is necessary to
   // get the matching correct in some cases.
   BaseReg = BaseReg ? BaseReg : 1;
   return X86Operand::CreateMem(SegReg, Disp, BaseReg, IndexReg, Scale, Start,
                                End, Size, Identifier, Info.OpDecl);

 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) {
+    Identifier = SemaCallback->LookupInlineAsmLabel(Identifier, getSourceManager(), Loc, false);
+    assert(Identifier.size() && "We should have an internal name here.");
+    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>