[ms] [llvm-ml] Add support for anonymous labels (`@@`, `@B`, `@F`)

[epastor]

Summary

ml.exe and ml64.exe support the use of anonymous labels, with @B and @F referring to the previous and next anonymous label respectively.

We add similar support to llvm-ml, with the exception that we are unable to emit an error message for an @F expression not followed by a @@ label.

[rnk]

Should these be unnamed labels so they don't appear in the COFF file symbol table? I'm guessing ml64 doesn't produce symbols for these labels, but correct me if I'm wrong.

If that is the case, I suggest:

• keep the counter
• store a vector of MCSymbols indexed by label counter
• use a getOrCreate operation to fill in that vector or read from it to handle support for forward references (@F)

llvm/lib/MC/MCParser/MasmParser.cpp

406

Looks like an unintended change

[epastor]

Switch to the pre-existing anonymous label support. (Missed this in the first attempt!)

[epastor]

In D128944#3623568, @rnk wrote:

Should these be unnamed labels so they don't appear in the COFF file symbol table? I'm guessing ml64 doesn't produce symbols for these labels, but correct me if I'm wrong.

If that is the case, I suggest:

• keep the counter
• store a vector of MCSymbols indexed by label counter
• use a getOrCreate operation to fill in that vector or read from it to handle support for forward references (@F)

It turns out we've already got support for directional local labels inside of MCContext; the difference here is that MASM only supports one set of local labels, where GNU-style assembler supports an integer-indexed family of them. I've revised this change to take advantage of the pre-existing support instead!

EDIT: And it looks like these will not be emitted to the symbol table, either.

llvm/lib/MC/MCParser/MasmParser.cpp

406

Removed, thanks.

[epastor]

Rebase to HEAD

[rnk]

lgtm, thanks!

cc +@ayzhao

llvm/lib/MC/MCParser/MasmParser.cpp

2250

Can IDVal be used directly in place of Label below to simplify?

[epastor]

Addressing last comment

[epastor]

Small update to comments

[epastor]

Fix collision with tombstone entry, and also remove one more bit of handling for GNU-as-style directional labels

llvm/lib/MC/MCParser/MasmParser.cpp

private:
   StringMap<Variable> Variables;
 
   /// Stack of active struct definitions.
   SmallVector<StructInfo, 1> StructInProgress;
 
   /// Maps struct tags to struct definitions.
   StringMap<StructInfo> Structs;
 
   /// Maps data location names to types.
   StringMap<AsmTypeInfo> KnownType;
 
   /// Stack of active macro instantiations.
   std::vector<MacroInstantiation*> ActiveMacros;
 
   /// List of bodies of anonymous macros.
   std::deque<MCAsmMacro> MacroLikeBodies;
 
case AsmToken::Identifier: {
     }
     // Parse named bitwise negation.
     if (Identifier.equals_insensitive("not")) {
       if (parsePrimaryExpr(Res, EndLoc, nullptr))
         return true;
       Res = MCUnaryExpr::createNot(Res, getContext(), FirstTokenLoc);
       return false;
     }
+    // Parse directional local label references.
+    if (Identifier.equals_insensitive("@b") ||
+        Identifier.equals_insensitive("@f")) {
+      bool Before = Identifier.equals_insensitive("@b");
+      MCSymbol *Sym = getContext().getDirectionalLocalSymbol(0, Before);
+      if (Before && Sym->isUndefined())
+        return Error(FirstTokenLoc, "Expected @@ label before @B reference");
+      Res = MCSymbolRefExpr::create(Sym, getContext());
+      return false;
+    }
     // Parse symbol variant.
     std::pair<StringRef, StringRef> Split;
     if (!MAI.useParensForSymbolVariant()) {
       if (FirstTokenKind == AsmToken::String) {
         if (Lexer.is(AsmToken::At)) {
           Lex(); // eat @
           SMLoc AtLoc = getLexer().getLoc();
           StringRef VName;
if (TypeInfo) {
 
       *TypeInfo = Info.Type;
     }
     return false;
   }
   case AsmToken::BigNum:
     return TokError("literal value out of range for directive");
   case AsmToken::Integer: {
-    SMLoc Loc = getTok().getLoc();
     int64_t IntVal = getTok().getIntVal();
     Res = MCConstantExpr::create(IntVal, getContext());
     EndLoc = Lexer.getTok().getEndLoc();
     Lex(); // Eat token.
-    // Look for 'b' or 'f' following an Integer as a directional label.
-    if (Lexer.getKind() == AsmToken::Identifier) {
-      StringRef IDVal = getTok().getString();
-      // Look up the symbol variant if used.
-      std::pair<StringRef, StringRef> Split = IDVal.split('@');
-      MCSymbolRefExpr::VariantKind Variant = MCSymbolRefExpr::VK_None;
-      if (Split.first.size() != IDVal.size()) {
-        Variant = MCSymbolRefExpr::getVariantKindForName(Split.second);
-        if (Variant == MCSymbolRefExpr::VK_Invalid)
-          return TokError("invalid variant '" + Split.second + "'");
-        IDVal = Split.first;
-      }
-      if (IDVal == "f" || IDVal == "b") {
-        MCSymbol *Sym =
-            Ctx.getDirectionalLocalSymbol(IntVal, IDVal == "b");
-        Res = MCSymbolRefExpr::create(Sym, Variant, getContext());
-        if (IDVal == "b" && Sym->isUndefined())
-          return Error(Loc, "directional label undefined");
-        DirLabels.push_back(std::make_tuple(Loc, CppHashInfo, Sym));
-        EndLoc = Lexer.getTok().getEndLoc();
-        Lex(); // Eat identifier.
-      }
-    }
     return false;
   }
   case AsmToken::String: {
     // MASM strings (used as constants) are interpreted as big-endian base-256.
     SMLoc ValueLoc = getTok().getLoc();
     std::string Value;
     if (parseEscapedString(Value))
       return true;
if (parseToken(AsmToken::Percent) || expandStatement(ExpansionLoc))
       return true;
   }
 
   // Statements always start with an identifier, unless we're dealing with a
   // processor directive (.386, .686, etc.) that lexes as a real.
   AsmToken ID = getTok();
   SMLoc IDLoc = ID.getLoc();
   StringRef IDVal;
-  int64_t LocalLabelVal = -1;
   if (Lexer.is(AsmToken::HashDirective))
     return parseCppHashLineFilenameComment(IDLoc);
-  // Allow an integer followed by a ':' as a directional local label.
-  if (Lexer.is(AsmToken::Integer)) {
-    LocalLabelVal = getTok().getIntVal();
-    if (LocalLabelVal < 0) {
-      if (!TheCondState.Ignore) {
-        Lex(); // always eat a token
-        return Error(IDLoc, "unexpected token at start of statement");
-      }
-      IDVal = "";
-    } else {
-      IDVal = getTok().getString();
-      Lex(); // Consume the integer token to be used as an identifier token.
-      if (Lexer.getKind() != AsmToken::Colon) {
-        if (!TheCondState.Ignore) {
-          Lex(); // always eat a token
-          return Error(IDLoc, "unexpected token at start of statement");
-        }
-      }
-    }
-  } else if (Lexer.is(AsmToken::Dot)) {
+  if (Lexer.is(AsmToken::Dot)) {
     // Treat '.' as a valid identifier in this context.
     Lex();
     IDVal = ".";
   } else if (Lexer.is(AsmToken::LCurly)) {
     // Treat '{' as a valid identifier in this context.
     Lex();
     IDVal = "{";
 
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 (ParsingMSInlineAsm && SI) {
-        StringRef RewrittenLabel =
-            SI->LookupInlineAsmLabel(IDVal, getSourceManager(), IDLoc, true);
-        assert(!RewrittenLabel.empty() &&
-               "We should have an internal name here.");
-        Info.AsmRewrites->emplace_back(AOK_Label, IDLoc, IDVal.size(),
-                                       RewrittenLabel);
-        IDVal = RewrittenLabel;
-      }
+    // Handle directional local labels
+    if (IDVal == "@@") {
+      Sym = Ctx.createDirectionalLocalSymbol(0);
+    } else {
       Sym = getContext().getOrCreateSymbol(IDVal);
-    } else
-      Sym = Ctx.createDirectionalLocalSymbol(LocalLabelVal);
+    }
+
     // End of Labels should be treated as end of line for lexing
     // purposes but that information is not available to the Lexer who
     // does not understand Labels. This may cause us to see a Hash
     // here instead of a preprocessor line comment.
     if (getTok().is(AsmToken::Hash)) {
       std::string CommentStr = parseStringTo(AsmToken::EndOfStatement);
       Lexer.Lex();
       Lexer.UnLex(AsmToken(AsmToken::EndOfStatement, CommentStr));
     }
 
     // Consume any end of statement token, if present, to avoid spurious
     // addBlankLine calls().
     if (getTok().is(AsmToken::EndOfStatement)) {
       Lex();
     }
 
     getTargetParser().doBeforeLabelEmit(Sym);

llvm/test/tools/llvm-ml/anonymous_labels.asm

+; RUN: llvm-ml -filetype=s %s /Fo - | FileCheck %s
+
+.code
+
+t1:
+  jmp @F
+  jmp @F
+; CHECK-LABEL: t1:
+; CHECK-NEXT: jmp [[TEMP1:[[:alpha:][:digit:]]+]]
+; CHECK-NEXT: jmp [[TEMP1]]
+
+@@:
+  xor eax, eax
+; CHECK: [[TEMP1]]:
+; CHECK-NEXT: xor eax, eax
+
+t2:
+  jmp @B
+  jmp @B
+; CHECK-LABEL: t2:
+; CHECK-NEXT: jmp [[TEMP1]]
+; CHECK-NEXT: jmp [[TEMP1]]
+
+t3:
+  jmp @F
+; CHECK-LABEL: t3:
+; CHECK-NEXT: jmp [[TEMP2:[[:alpha:][:digit:]]+]]
+
+@@:
+  xor eax, eax
+; CHECK: [[TEMP2]]:
+; CHECK-NEXT: xor eax, eax
+
+@@:
+  xor eax, eax
+; CHECK: [[TEMP3:[[:alpha:][:digit:]]+]]:
+; CHECK-NEXT: xor eax, eax
+
+t4:
+  jmp @B
+; CHECK-LABEL: t4:
+; CHECK-NEXT: jmp [[TEMP3]]

llvm/test/tools/llvm-ml/anonymous_labels_errors.asm

+; RUN: not llvm-ml -filetype=s %s /Fo - 2>&1 | FileCheck %s --implicit-check-not=error:
+
+.code
+
+; CHECK: :[[# @LINE + 2]]:5: error: Expected @@ label before @B reference
+; CHECK: :[[# @LINE + 1]]:7: error: Unexpected identifier!
+jmp @B
+
+@@:
+  jmp @B
+  jmp @F
+@@:
+  xor eax, eax
+
+; NOTE: a trailing @F will not fail; fixing this seems to require two passes.
+jmp @F