ms-inline-asm: Fix parsing label names inside bracket expressions

[• ehsan]

Summary

This fixes a couple of issues. One is ensuring that AOK_Label rewrite
rules have a lower priority than AOK_Skip rules, as AOK_Skip needs to
be able to skip the brackets properly. The other part of the fix ensures
that we don't overwrite Identifier when looking up the identifier, and
that we use the locally available information to generate the AOK_Label
rewrite in ParseIntelIdentifier. Doing that in CreateMemForInlineAsm
would be problematic since the Start location there may point to the
beginning of a bracket expression, and not necessarily the beginning of
an identifier.

This also means that we don't need to carry around the InternlName field,
which helps simplify the code.

[rnk]

lgtm

lib/Target/X86/AsmParser/X86AsmParser.cpp

1344 ↗

(On Diff #13949)

80cols

[• ehsan]

Closed by commit rL218270 (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,

llvm/trunk/include/llvm/MC/MCTargetAsmParser.h

enum AsmRewriteKind {
   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
+  2, // AOK_Align
+  2, // AOK_DotOperator
+  2, // AOK_Emit
+  4, // AOK_Imm
+  4, // AOK_ImmPrefix
+  3, // AOK_Input
+  3, // AOK_Output
+  5, // AOK_SizeDirective
   1, // AOK_Label
-  1  // AOK_Skip
+  2  // AOK_Skip
 };
 
 struct AsmRewrite {
   AsmRewriteKind Kind;
   SMLoc Loc;
   unsigned Len;
   unsigned Val;
   StringRef Label;

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);
while (true) {
     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;
+    StringRef InternalName =
+      SemaCallback->LookupInlineAsmLabel(Identifier, getSourceManager(),
+                                         Loc, false);
+    assert(InternalName.size() && "We should have an internal name here.");
+    // Push a rewrite for replacing the identifier name with the internal name.
+    InstInfo->AsmRewrites->push_back(AsmRewrite(AOK_Label, Loc,
+                                                Identifier.size(),
+                                                InternalName));
   }
 
   // Create the symbol reference.
   MCSymbol *Sym = getContext().GetOrCreateSymbol(Identifier);
   MCSymbolRefExpr::VariantKind Variant = MCSymbolRefExpr::VK_None;
   Val = MCSymbolRefExpr::Create(Sym, Variant, getParser().getContext());
   return false;
 }