[ms] [llvm-ml] Improve data support, adding names and complex initializers.

[epastor]

Summary

Add support for ?, DUP, and string initializers, as well as MASM syntax for named data locations.

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, console-log.txt

[epastor]

Rebase

[epastor]

Add support for complex real-valued initializers

[epastor]

Rebase

[epastor]

Correct for casing & casting changes in previous rebase

[epastor]

Fix several clang-tidy and clang-format suggestions

[thakis]

llvm/lib/MC/MCParser/MasmParser.cpp

2889

What does ml do with

BYTE 2, 4, 6, 8,
BYTE 2, 4, 6, 8,

? Does it really error out?

[epastor]

llvm/lib/MC/MCParser/MasmParser.cpp

2889

Believe it or not, yes!

test.asm(2) : error A2206:missing operator in expression

[thakis]

llvm/lib/MC/MCParser/MasmParser.cpp

3018

Is there no more efficient way to express this internally? If you 1000000 dup (2), do we have to write out 1M 2s?

[epastor]

llvm/lib/MC/MCParser/MasmParser.cpp

3018

We could optimize the case of 1000000 dup (2) with a fill-type instruction, but I'm not sure how we would handle (e.g.) 500000 dup (2,3). Also, this is data-initialization logic, so either way the actual binary is going to contain 1M literal copies. Do you think it's worth special-casing the single-value handling internally anyway?

[thakis]

llvm/lib/MC/MCParser/MasmParser.cpp

3018

as-is is fine for now

[epastor]

llvm/lib/MC/MCParser/MasmParser.cpp

3018

Just a note for the future. Currently this means we actually make 1M copies of the APInt representing a real with value 2 in memory while compiling, and don't let go until we've processed the entire initializer list.

There are three cases here:

1. 1000000 dup (2) - writes out 2,2,2,2,... to 1M positions.
2. 500000 dup (2,3) - writes out 2,3,2,3,... to 1M positions.
3. 250000 dup (2, 3 dup (1)) - writes out 2,1,1,1,2,1,1,1,... to 1M positions.

Case 1 could be optimized by using Fill logic.
Case 2 could be handled by passing a "duplication count" to the recursive call, and letting it emit results directly.

Case 3 is more of a problem, though. Arbitrarily nested duplications get hard unless you expand them in place, as the current code does... or unless you parse them into a specialized tree structure, and expand it back to actual values only as needed.

All of this is complicated by the fact that once STRUCT support is in place, we do need to be able to parse initializers and hold them in memory, not just emit them immediately.

The ideal solution would probably be a tree-based intermediate representation for initializers, with duplication nodes representing "X DUP (<child>)". It's probably worth coming back to this eventually, but we'll leave it as-is for now.

[stella.stamenova]

It looks like this broke the Windows LLDB Buildbot:

http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/14117

[epastor]

In D73226#1890054, @stella.stamenova wrote:

It looks like this broke the Windows LLDB Buildbot:

http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/14117

Already reverted in http://reviews.llvm.org/rG9fe769a961dc - apologies, I lost track and thought we'd moved to supporting C++17 code in the codebase. The version here included use of an initializer in an if-statement.

This will be landed again in another commit.

llvm/lib/MC/MCParser/MasmParser.cpp

enum CVDefRangeType {
     CVDR_DEFRANGE_SUBFIELD_REGISTER,
     CVDR_DEFRANGE_REGISTER_REL
   };
 
   /// Maps Codeview def_range types --> CVDefRangeType enum, for
   /// Codeview def_range types parsed by this class.
   StringMap<CVDefRangeType> CVDefRangeTypeMap;
 
+  bool parseInitValue(unsigned Size);
+
   // ".ascii", ".asciz", ".string"
   bool parseDirectiveAscii(StringRef IDVal, bool ZeroTerminated);
-  bool parseDirectiveValue(StringRef IDVal,
-                           unsigned Size);       // "byte", "word", ...
-  bool parseDirectiveRealValue(StringRef IDVal,
-                               const fltSemantics &);  // "real4", ...
+
+  // "byte", "word", ...
+  bool parseScalarInstList(unsigned Size,
+                           SmallVectorImpl<const MCExpr *> &Values);
+  bool parseDirectiveValue(StringRef IDVal, unsigned Size);
+  bool parseDirectiveNamedValue(StringRef IDVal, unsigned Size, StringRef Name,
+                                SMLoc NameLoc);
+
+  // "real4", "real8"
+  bool parseDirectiveRealValue(StringRef IDVal, const fltSemantics &Semantics);
+  bool parseRealInstList(const fltSemantics &Semantics,
+                         SmallVectorImpl<APInt> &Values);
+  bool parseDirectiveNamedRealValue(StringRef IDVal,
+                                    const fltSemantics &Semantics,
+                                    StringRef Name, SMLoc NameLoc);
 
   // "=", "equ", "textequ"
   bool parseDirectiveEquate(StringRef IDVal, StringRef Name,
                             DirectiveKind DirKind);
 
   bool parseDirectiveOrg(); // ".org"
   bool parseDirectiveAlign();  // "align"
 
bool MasmParser::parseStatement(ParseStatementInfo &Info,
   switch (DirKind) {
   default:
     break;
   case DK_ASSIGN:
   case DK_EQU:
   case DK_TEXTEQU:
     Lex();
     return parseDirectiveEquate(nextVal, IDVal, DirKind);
+  case DK_BYTE:
+  case DK_DB:
+    Lex();
+    return parseDirectiveNamedValue(nextVal, 1, IDVal, IDLoc);
+  case DK_WORD:
+  case DK_DW:
+    Lex();
+    return parseDirectiveNamedValue(nextVal, 2, IDVal, IDLoc);
+  case DK_DWORD:
+  case DK_DD:
+    Lex();
+    return parseDirectiveNamedValue(nextVal, 4, IDVal, IDLoc);
+  case DK_FWORD:
+    Lex();
+    return parseDirectiveNamedValue(nextVal, 6, IDVal, IDLoc);
+  case DK_QWORD:
+  case DK_DQ:
+    Lex();
+    return parseDirectiveNamedValue(nextVal, 8, IDVal, IDLoc);
+  case DK_REAL4:
+    Lex();
+    return parseDirectiveNamedRealValue(nextVal, APFloat::IEEEsingle(), IDVal,
+                                        IDLoc);
+  case DK_REAL8:
+    Lex();
+    return parseDirectiveNamedRealValue(nextVal, APFloat::IEEEdouble(), IDVal,
+                                        IDLoc);
   }
 
   // __asm _emit or __asm __emit
   if (ParsingInlineAsm && (IDVal == "_emit" || IDVal == "__emit" ||
                            IDVal == "_EMIT" || IDVal == "__EMIT"))
     return parseDirectiveMSEmit(IDLoc, Info, IDVal.size());
 
   // __asm align
auto parseOp = [&]() -> bool {
     return false;
   };
 
   if (parseMany(parseOp))
     return addErrorSuffix(" in '" + Twine(IDVal) + "' directive");
   return false;
 }
 
-/// parseDirectiveValue
-///  ::= (byte | word | ... ) [ expression (, expression)* ]
-bool MasmParser::parseDirectiveValue(StringRef IDVal, unsigned Size) {
-  auto parseOp = [&]() -> bool {
+bool MasmParser::parseScalarInstList(unsigned Size,
+                                     SmallVectorImpl<const MCExpr *> &Values) {
+  do {
+    if (getTok().is(AsmToken::String)) {
+      StringRef Value = getTok().getStringContents();
+      if (Size == 1) {
+        // Treat each character as an initializer.
+        for (const char CharVal : Value)
+          Values.push_back(MCConstantExpr::create(CharVal, getContext()));
+      } else {
+        // Treat the string as an initial value in big-endian representation.
+        if (Value.size() > Size)
+          return Error(getTok().getLoc(), "out of range literal value");
+
+        uint64_t IntValue = 0;
+        for (const unsigned char CharVal : Value.bytes())
+          IntValue = (IntValue << 8) | CharVal;
+        Values.push_back(MCConstantExpr::create(IntValue, getContext()));
+      }
+      Lex();
+    } else {
       const MCExpr *Value;
-    SMLoc ExprLoc = getLexer().getLoc();
       if (checkForValidSection() || parseExpression(Value))
         return true;
+      if (getTok().is(AsmToken::Identifier) &&
+          getTok().getString().equals_lower("dup")) {
+        Lex();  // eat 'dup'
+        const MCConstantExpr *MCE = dyn_cast<MCConstantExpr>(Value);
+        if (!MCE)
+          return Error(Value->getLoc(),
+                       "cannot repeat value a non-constant number of times");
+        const int64_t Repetitions = MCE->getValue();
+        if (Repetitions < 0)
+          return Error(Value->getLoc(),
+                       "cannot repeat value a negative number of times");
+
+        SmallVector<const MCExpr *, 1> DuplicatedValues;
+        if (parseToken(AsmToken::LParen,
+                       "parentheses required for 'dup' contents") ||
+            parseScalarInstList(Size, DuplicatedValues) ||
+            parseToken(AsmToken::RParen, "unmatched parentheses"))
+          return true;
+
+        for (int i = 0; i < Repetitions; ++i)
+          Values.append(DuplicatedValues.begin(), DuplicatedValues.end());
+      } else {
+        Values.push_back(Value);
+      }
+    }
+
+    // Continue if we see a comma. (Also, allow line continuation.)
+  } while (parseOptionalToken(AsmToken::Comma) &&
+           (getTok().isNot(AsmToken::EndOfStatement) ||
+            !parseToken(AsmToken::EndOfStatement)));
+
+  return false;
+}
+
+/// parseDirectiveValue
+///  ::= (byte | word | ... ) [ expression (, expression)* ]
+bool MasmParser::parseDirectiveValue(StringRef IDVal, unsigned Size) {
+  SmallVector<const MCExpr *, 1> Values;
+  if (parseScalarInstList(Size, Values))
+    return addErrorSuffix(" in '" + Twine(IDVal) + "' directive");
+
+  for (const MCExpr *Value : Values) {
     // Special case constant expressions to match code generator.
     if (const MCConstantExpr *MCE = dyn_cast<MCConstantExpr>(Value)) {
       assert(Size <= 8 && "Invalid size");
       int64_t IntValue = MCE->getValue();
       if (!isUIntN(8 * Size, IntValue) && !isIntN(8 * Size, IntValue))
-        return Error(ExprLoc, "out of range literal value");
+        return Error(MCE->getLoc(), "out of range literal value");
       getStreamer().emitIntValue(IntValue, Size);
-    } else
-      getStreamer().emitValue(Value, Size, ExprLoc);
-    return false;
-  };
+    } else if (const MCSymbolRefExpr *MSE = dyn_cast<MCSymbolRefExpr>(Value);
+               MSE && MSE->getSymbol().getName() == "?") {
+      // ? initializer; treat as 0.
+      getStreamer().emitIntValue(0, Size);
+    } else {
+      getStreamer().emitValue(Value, Size, Value->getLoc());
+    }
+  }
+}
 
-  if (parseMany(parseOp))
-    return addErrorSuffix(" in '" + Twine(IDVal) + "' directive");
-  return false;
+/// parseDirectiveNamedValue
+///  ::= name (byte | word | ... ) [ expression (, expression)* ]
+bool MasmParser::parseDirectiveNamedValue(StringRef IDVal, unsigned Size,
+                                          StringRef Name, SMLoc NameLoc) {
+  MCSymbol *Sym = getContext().getOrCreateSymbol(Name);
+  getStreamer().emitLabel(Sym);
+  return parseDirectiveValue(IDVal, Size);
 }
 
 static bool parseHexOcta(MasmParser &Asm, uint64_t &hi, uint64_t &lo) {
   if (Asm.getTok().isNot(AsmToken::Integer) &&
       Asm.getTok().isNot(AsmToken::BigNum))
     return Asm.TokError("unknown token in expression");
   SMLoc ExprLoc = Asm.getTok().getLoc();
   APInt IntValue = Asm.getTok().getAPIntVal();
bool MasmParser::parseRealValue(const fltSemantics &Semantics, APInt &Res) {
   // Consume the numeric token.
   Lex();
 
   Res = Value.bitcastToAPInt();
 
   return false;
 }
 
+bool MasmParser::parseRealInstList(const fltSemantics &Semantics,
+                                   SmallVectorImpl<APInt> &ValuesAsInt) {
+  do {
+    const AsmToken NextTok = Lexer.peekTok();
+    if (NextTok.is(AsmToken::Identifier) &&
+        NextTok.getString().equals_lower("dup")) {
+      const MCExpr *Value;
+      if (parseExpression(Value) || parseToken(AsmToken::Identifier))
+        return true;
+      const MCConstantExpr *MCE = dyn_cast<MCConstantExpr>(Value);
+      if (!MCE)
+        return Error(Value->getLoc(),
+                     "cannot repeat value a non-constant number of times");
+      const int64_t Repetitions = MCE->getValue();
+      if (Repetitions < 0)
+        return Error(Value->getLoc(),
+                     "cannot repeat value a negative number of times");
+
+      SmallVector<APInt, 1> DuplicatedValues;
+      if (parseToken(AsmToken::LParen,
+                     "parentheses required for 'dup' contents") ||
+          parseRealInstList(Semantics, DuplicatedValues) ||
+          parseToken(AsmToken::RParen, "unmatched parentheses"))
+        return true;
+
+      for (int i = 0; i < Repetitions; ++i)
+        ValuesAsInt.append(DuplicatedValues.begin(), DuplicatedValues.end());
+    } else {
+      APInt AsInt;
+      if (parseRealValue(Semantics, AsInt))
+        return true;
+      ValuesAsInt.push_back(AsInt);
+    }
+    // Continue if we see a comma. (Also, allow line continuation.)
+  } while (parseOptionalToken(AsmToken::Comma) &&
+           (getTok().isNot(AsmToken::EndOfStatement) ||
+            !parseToken(AsmToken::EndOfStatement)));
+
+  return false;
+}
+
 /// parseDirectiveRealValue
 ///  ::= (real4 | real8) [ expression (, expression)* ]
 bool MasmParser::parseDirectiveRealValue(StringRef IDVal,
                                          const fltSemantics &Semantics) {
-  auto parseOp = [&]() -> bool {
-    APInt AsInt;
-    if (checkForValidSection() || parseRealValue(Semantics, AsInt))
-      return true;
+  if (checkForValidSection())
+
+  SmallVector<APInt, 1> ValuesAsInt;
+  if (parseRealInstList(Semantics, ValuesAsInt))
+    return addErrorSuffix(" in '" + Twine(IDVal) + "' directive");
+
+  for (const APInt &AsInt : ValuesAsInt) {
     getStreamer().emitIntValue(AsInt.getLimitedValue(),
                                AsInt.getBitWidth() / 8);
-    return false;
-  };
+  }
+}
 
-  if (parseMany(parseOp))
-    return addErrorSuffix(" in '" + Twine(IDVal) + "' directive");
-  return false;
+/// parseDirectiveNamedRealValue
+///  ::= name (real4 | real8) [ expression (, expression)* ]
+bool MasmParser::parseDirectiveNamedRealValue(StringRef IDVal,
+                                              const fltSemantics &Semantics,
+                                              StringRef Name, SMLoc NameLoc) {
+  MCSymbol *Sym = getContext().getOrCreateSymbol(Name);
+  getStreamer().emitLabel(Sym);
+  return parseDirectiveRealValue(IDVal, Semantics);
 }
 
 /// parseDirectiveOrg
 ///  ::= .org expression [ , expression ]
 bool MasmParser::parseDirectiveOrg() {
   const MCExpr *Offset;
   SMLoc OffsetLoc = Lexer.getLoc();
   if (checkForValidSection() || parseExpression(Offset))

llvm/test/tools/llvm-ml/basic_data.test

+# RUN: llvm-ml -filetype=asm %s | FileCheck %s
+
+.data
+BYTE 2, 4, 6, 8
+; CHECK: .data
+; CHECK-NEXT: .byte 2
+; CHECK-NEXT: .byte 4
+; CHECK-NEXT: .byte 6
+; CHECK-NEXT: .byte 8
+
+BYTE 2 dup (1, 2 dup (2)),
+     3
+; CHECK: .byte  1
+; CHECK-NEXT: .byte 2
+; CHECK-NEXT: .byte 2
+; CHECK-NEXT: .byte 1
+; CHECK-NEXT: .byte 2
+; CHECK-NEXT: .byte 2
+; CHECK-NEXT: .byte 3
+
+REAL4 1, 0
+; CHECK: .long 1065353216
+; CHECK-NEXT: .long 0
+
+REAL4 2 DUP (2.5, 2 dup (0)),
+      4
+; CHECK: .long 1075838976
+; CHECK-NEXT: .long 0
+; CHECK-NEXT: .long 0
+; CHECK-NEXT: .long 1075838976
+; CHECK-NEXT: .long 0
+; CHECK-NEXT: .long 0
+; CHECK-NEXT: .long 1082130432
+
+.code
+BYTE 5
+; CHECK: .text
+; CHECK-NEXT: .byte 5