[ms] [llvm-ml] Add support for .radix directive, and accept all radix specifiers

[epastor]

Summary

Add support for .radix directive, and radix specifiers [yY] (binary), [oOqQ] (octal), and [tT] (decimal).

Also, when lexing MASM integers, require radix specifier; MASM requires that all literals without a radix specifier be treated as in the default radix. (e.g., 0100 = 100)

[thakis]

llvm/lib/MC/MCParser/AsmLexer.cpp

403

Can't hex literals contain 'e' in non-float literals as well? how is that handled?

[epastor]

Handle float literals in MASM earlier, and avoid confusing the hex digit 'e' for an exponential marker.

[epastor]

llvm/lib/MC/MCParser/AsmLexer.cpp

403

... that was a really good question, thank you! Apparently in MASM, float literals are always distinguished by containing a . character; it's invalid to write (e.g.) 13E1 unless the default radix is >= 15, and it will then be interpreted as an integer.

I've corrected the code.

[thakis]

llvm/lib/MC/MCParser/AsmLexer.cpp

403

Thanks! Can you add a test that would've failed with the old code but that passes with the new code?

[epastor]

Fix error in float literal lexing, and improve tests.

Also, improve lexer error handling for X86AsmParser in Intel expressions.

[thakis]

lg assuming we have tests for float literals containing an e in them somewhere already.

llvm/include/llvm/MC/MCParser/MCAsmLexer.h

class MCAsmLexer {
   std::string Err;
 
 protected: // Can only create subclasses.
   const char *TokStart = nullptr;
   bool SkipSpace = true;
   bool AllowAtInIdentifier;
   bool IsAtStartOfStatement = true;
   bool LexMasmIntegers = false;
+  unsigned DefaultRadix = 10;
   AsmCommentConsumer *CommentConsumer = nullptr;
 
   MCAsmLexer();
 
   virtual AsmToken LexToken() = 0;
 
   void SetError(SMLoc errLoc, const std::string &err) {
     ErrLoc = errLoc;
public:
   bool isNot(AsmToken::TokenKind K) const { return getTok().isNot(K); }
 
   /// Set whether spaces should be ignored by the lexer
   void setSkipSpace(bool val) { SkipSpace = val; }
 
   bool getAllowAtInIdentifier() { return AllowAtInIdentifier; }
   void setAllowAtInIdentifier(bool v) { AllowAtInIdentifier = v; }
 
+  unsigned getDefaultRadix() const { return DefaultRadix; }
+  void setDefaultRadix(unsigned Radix) { DefaultRadix = Radix; }
+
   void setCommentConsumer(AsmCommentConsumer *CommentConsumer) {
     this->CommentConsumer = CommentConsumer;
   }
 
   /// Set whether to lex masm-style binary and hex literals. They look like
   /// 0b1101 and 0ABCh respectively.
   void setLexMasmIntegers(bool V) { LexMasmIntegers = V; }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_MC_MCPARSER_MCASMLEXER_H

llvm/lib/MC/MCParser/AsmLexer.cpp

 //===- AsmLexer.cpp - Lexer for Assembly Files ----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This class implements the lexer for assembly files.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/MC/MCParser/AsmLexer.h"
+#include "third_party/llvm/llvm-project/llvm/include/llvm/Support/Compiler.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCParser/MCAsmLexer.h"
 #include "llvm/Support/SMLoc.h"
static unsigned doHexLookAhead(const char *&CurPtr, unsigned DefaultRadix,
   }
   bool isHex = LexHex && (*LookAhead == 'h' || *LookAhead == 'H');
   CurPtr = isHex || !FirstNonDec ? LookAhead : FirstNonDec;
   if (isHex)
     return 16;
   return DefaultRadix;
 }
 
+static const char *findLastDigit(const char *CurPtr, unsigned DefaultRadix) {
+  while (hexDigitValue(*CurPtr) < DefaultRadix) {
+    ++CurPtr;
+  }
+  return CurPtr;
+}
+
 static AsmToken intToken(StringRef Ref, APInt &Value)
 {
   if (Value.isIntN(64))
     return AsmToken(AsmToken::Integer, Ref, Value);
   return AsmToken(AsmToken::BigNum, Ref, Value);
 }
 
+static std::string radixName(unsigned Radix) {
+  switch (Radix) {
+  case 2:
+    return "binary";
+  case 8:
+    return "octal";
+  case 10:
+    return "decimal";
+  case 16:
+    return "hexadecimal";
+  default:
+    return "base-" + std::to_string(Radix);
+  }
+}
+
 /// LexDigit: First character is [0-9].
 ///   Local Label: [0-9][:]
 ///   Forward/Backward Label: [0-9][fb]
 ///   Binary integer: 0b[01]+
 ///   Octal integer: 0[0-7]+
 ///   Hex integer: 0x[0-9a-fA-F]+ or [0x]?[0-9][0-9a-fA-F]*[hH]
 ///   Decimal integer: [1-9][0-9]*
 AsmToken AsmLexer::LexDigit() {
-  // MASM-flavor binary integer: [01]+[bB]
+  // MASM-flavor binary integer: [01]+[yY] (if DefaultRadix < 16, [bByY])
+  // MASM-flavor octal integer: [0-7]+[oOqQ]
+  // MASM-flavor decimal integer: [0-9]+[tT] (if DefaultRadix < 16, [dDtT])
   // MASM-flavor hexadecimal integer: [0-9][0-9a-fA-F]*[hH]
   if (LexMasmIntegers && isdigit(CurPtr[-1])) {
-    const char *FirstNonBinary = (CurPtr[-1] != '0' && CurPtr[-1] != '1') ?
-                                   CurPtr - 1 : nullptr;
+    const char *FirstNonBinary =
+        (CurPtr[-1] != '0' && CurPtr[-1] != '1') ? CurPtr - 1 : nullptr;
+    const char *FirstNonDecimal =
+        (CurPtr[-1] < '0' || CurPtr[-1] > '9') ? CurPtr - 1 : nullptr;
     const char *OldCurPtr = CurPtr;
     while (isHexDigit(*CurPtr)) {
-      if (*CurPtr != '0' && *CurPtr != '1' && !FirstNonBinary)
+      switch (*CurPtr) {
+      default:
+        if (!FirstNonDecimal) {
+          FirstNonDecimal = CurPtr;
+        }
+        LLVM_FALLTHROUGH;
+      case '9':
+      case '8':
+      case '7':
+      case '6':
+      case '5':
+      case '4':
+      case '3':
+      case '2':
+        if (!FirstNonBinary) {
           FirstNonBinary = CurPtr;
+        }
+        break;
+      case '1':
+      case '0':
+        break;
+      }
       ++CurPtr;
     }
+    if (*CurPtr == '.') {
+      // MASM float literals (other than hex floats) always contain a ".", and
+      // are always written in decimal.
+      ++CurPtr;
+      return LexFloatLiteral();
+    }
 
     unsigned Radix = 0;
     if (*CurPtr == 'h' || *CurPtr == 'H') {
       // hexadecimal number
       ++CurPtr;
       Radix = 16;
+    } else if (*CurPtr == 't' || *CurPtr == 'T') {
+      // decimal number
+      ++CurPtr;
+      Radix = 10;
+    } else if (*CurPtr == 'o' || *CurPtr == 'O' || *CurPtr == 'q' ||
+               *CurPtr == 'Q') {
+      // octal number
+      ++CurPtr;
+      Radix = 8;
+    } else if (*CurPtr == 'y' || *CurPtr == 'Y') {
+      // binary number
+      ++CurPtr;
+      Radix = 2;
+    } else if (FirstNonDecimal && FirstNonDecimal + 1 == CurPtr &&
+               DefaultRadix < 14 &&
+               (*FirstNonDecimal == 'd' || *FirstNonDecimal == 'D')) {
+      Radix = 10;
     } else if (FirstNonBinary && FirstNonBinary + 1 == CurPtr &&
-               (*FirstNonBinary == 'b' || *FirstNonBinary == 'B'))
+               DefaultRadix < 12 &&
+               (*FirstNonBinary == 'b' || *FirstNonBinary == 'B')) {
       Radix = 2;
+    }
 
-    if (Radix == 2 || Radix == 16) {
+    if (Radix) {
       StringRef Result(TokStart, CurPtr - TokStart);
       APInt Value(128, 0, true);
 
       if (Result.drop_back().getAsInteger(Radix, Value))
-        return ReturnError(TokStart, Radix == 2 ? "invalid binary number" :
-                             "invalid hexdecimal number");
+        return ReturnError(TokStart, "invalid " + radixName(Radix) + " number");
 
       // MSVC accepts and ignores type suffices on integer literals.
       SkipIgnoredIntegerSuffix(CurPtr);
 
       return intToken(Result, Value);
     }
 
-    // octal/decimal integers, or floating point numbers, fall through
+    // default-radix integers, or floating point numbers, fall through
     CurPtr = OldCurPtr;
   }
 
+  // MASM default-radix integers: [0-9a-fA-F]+
+  // (All other integer literals have a radix specifier.)
+  if (LexMasmIntegers) {
+    CurPtr = findLastDigit(CurPtr, 16);
+    StringRef Result(TokStart, CurPtr - TokStart);
+
+    APInt Value(128, 0, true);
+    if (Result.getAsInteger(DefaultRadix, Value)) {
+      return ReturnError(TokStart,
+                         "invalid " + radixName(DefaultRadix) + " number");
+    }
+
+    return intToken(Result, Value);
+  }
+
   // Decimal integer: [1-9][0-9]*
   if (CurPtr[-1] != '0' || CurPtr[0] == '.') {
     unsigned Radix = doHexLookAhead(CurPtr, 10, LexMasmIntegers);
     bool isHex = Radix == 16;
     // Check for floating point literals.
     if (!isHex && (*CurPtr == '.' || *CurPtr == 'e' || *CurPtr == 'E')) {
       if (*CurPtr == '.')
         ++CurPtr;
       return LexFloatLiteral();
     }
 
     StringRef Result(TokStart, CurPtr - TokStart);
 
     APInt Value(128, 0, true);
-    if (Result.getAsInteger(Radix, Value))
-      return ReturnError(TokStart, !isHex ? "invalid decimal number" :
-                           "invalid hexdecimal number");
-
-    // Consume the [hH].
-    if (LexMasmIntegers && Radix == 16)
-      ++CurPtr;
+    if (Result.getAsInteger(Radix, Value)) {
+      return ReturnError(TokStart, "invalid " + radixName(Radix) + " number");
+    }
 
     // The darwin/x86 (and x86-64) assembler accepts and ignores type
     // suffices on integer literals.
     SkipIgnoredIntegerSuffix(CurPtr);
 
     return intToken(Result, Value);
   }
 
if ((*CurPtr == 'x') || (*CurPtr == 'X')) {
     SkipIgnoredIntegerSuffix(CurPtr);
 
     return intToken(StringRef(TokStart, CurPtr - TokStart), Result);
   }
 
   // Either octal or hexadecimal.
   APInt Value(128, 0, true);
   unsigned Radix = doHexLookAhead(CurPtr, 8, LexMasmIntegers);
-  bool isHex = Radix == 16;
   StringRef Result(TokStart, CurPtr - TokStart);
   if (Result.getAsInteger(Radix, Value))
-    return ReturnError(TokStart, !isHex ? "invalid octal number" :
-                       "invalid hexdecimal number");
+    return ReturnError(TokStart, "invalid " + radixName(Radix) + " number");
 
   // Consume the [hH].
   if (Radix == 16)
     ++CurPtr;
 
   // The darwin/x86 (and x86-64) assembler accepts and ignores ULL and LL
   // suffixes on integer literals.
   SkipIgnoredIntegerSuffix(CurPtr);

llvm/lib/MC/MCParser/COFFMasmParser.cpp

void Initialize(MCAsmParser &Parser) override {
     // alias
     // assume
     // .fpo
     addDirectiveHandler<&COFFMasmParser::ParseDirectiveIncludelib>(
         "includelib");
     // option
     // popcontext
     // pushcontext
-    // .radix
     // .safeseh
 
     // Procedure directives
     addDirectiveHandler<&COFFMasmParser::ParseDirectiveEndProc>("endp");
     // invoke (32-bit only)
     addDirectiveHandler<&COFFMasmParser::ParseDirectiveProc>("proc");
     // proto
 

llvm/lib/MC/MCParser/MasmParser.cpp

enum DirectiveKind {
     DK_UNION,
     DK_ENDS,
     DK_END,
     DK_PUSHFRAME,
     DK_PUSHREG,
     DK_SAVEREG,
     DK_SAVEXMM128,
     DK_SETFRAME,
+    DK_RADIX,
   };
 
   /// Maps directive name --> DirectiveKind enum, for directives parsed by this
   /// class.
   StringMap<DirectiveKind> DirectiveKindMap;
 
   // Codeview def_range type parsing.
   enum CVDefRangeType {
private:
   bool parseDirectiveErrorIfdef(SMLoc DirectiveLoc, bool ExpectDefined);
   // ".erridn", ".errdif", ".erridni", or ".errdifi", depending on ExpectEqual
   // and CaseInsensitive.
   bool parseDirectiveErrorIfidn(SMLoc DirectiveLoc, bool ExpectEqual,
                                 bool CaseInsensitive);
   // ".erre" or ".errnz", depending on ExpectZero.
   bool parseDirectiveErrorIfe(SMLoc DirectiveLoc, bool ExpectZero);
 
+  // ".radix"
+  bool parseDirectiveRadix(SMLoc DirectiveLoc);
+
   // "echo"
   bool parseDirectiveEcho();
 
   void initializeDirectiveKindMap();
   void initializeCVDefRangeTypeMap();
 };
 
 } // end anonymous namespace
case DK_ERRIDN:
                                       /*CaseInsensitive=*/false);
     case DK_ERRIDNI:
       return parseDirectiveErrorIfidn(IDLoc, /*ExpectEqual=*/true,
                                       /*CaseInsensitive=*/true);
     case DK_ERRE:
       return parseDirectiveErrorIfe(IDLoc, true);
     case DK_ERRNZ:
       return parseDirectiveErrorIfe(IDLoc, false);
+    case DK_RADIX:
+      return parseDirectiveRadix(IDLoc);
     case DK_ECHO:
       return parseDirectiveEcho();
     }
 
     return Error(IDLoc, "unknown directive");
   }
 
   // We also check if this is allocating memory with user-defined type.
void MasmParser::initializeDirectiveKindMap() {
   DirectiveKindMap[".erridni"] = DK_ERRIDNI;
   DirectiveKindMap[".erre"] = DK_ERRE;
   DirectiveKindMap[".errnz"] = DK_ERRNZ;
   DirectiveKindMap[".pushframe"] = DK_PUSHFRAME;
   DirectiveKindMap[".pushreg"] = DK_PUSHREG;
   DirectiveKindMap[".savereg"] = DK_SAVEREG;
   DirectiveKindMap[".savexmm128"] = DK_SAVEXMM128;
   DirectiveKindMap[".setframe"] = DK_SETFRAME;
+  DirectiveKindMap[".radix"] = DK_RADIX;
   // DirectiveKindMap[".altmacro"] = DK_ALTMACRO;
   // DirectiveKindMap[".noaltmacro"] = DK_NOALTMACRO;
   DirectiveKindMap["db"] = DK_DB;
   DirectiveKindMap["dd"] = DK_DD;
   DirectiveKindMap["df"] = DK_DF;
   DirectiveKindMap["dq"] = DK_DQ;
   DirectiveKindMap["dw"] = DK_DW;
   DirectiveKindMap["echo"] = DK_ECHO;
bool MasmParser::parseDirectiveMSAlign(SMLoc IDLoc, ParseStatementInfo &Info) {
   uint64_t IntValue = MCE->getValue();
   if (!isPowerOf2_64(IntValue))
     return Error(ExprLoc, "literal value not a power of two greater then zero");
 
   Info.AsmRewrites->emplace_back(AOK_Align, IDLoc, 5, Log2_64(IntValue));
   return false;
 }
 
+bool MasmParser::parseDirectiveRadix(SMLoc DirectiveLoc) {
+  const SMLoc Loc = getLexer().getLoc();
+  StringRef RadixString = parseStringToEndOfStatement().trim();
+  unsigned Radix;
+  if (RadixString.getAsInteger(10, Radix)) {
+    return Error(Loc,
+                 "radix must be a decimal number in the range 2 to 16; was " +
+                     RadixString);
+  }
+  if (Radix < 2 || Radix > 16)
+    return Error(Loc, "radix must be in the range 2 to 16; was " +
+                          std::to_string(Radix));
+  getLexer().setDefaultRadix(Radix);
+  return false;
+}
+
 bool MasmParser::parseDirectiveEcho() {
   StringRef Message = parseStringToEndOfStatement();
   Lex();  // eat end of statement
   llvm::outs() << Message << '\n';
   return false;
 }
 
 // We are comparing pointers, but the pointers are relative to a single string.

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

while (!Done) {
     bool UpdateLocLex = true;
     AsmToken::TokenKind TK = getLexer().getKind();
 
     switch (TK) {
     default:
       if ((Done = SM.isValidEndState()))
         break;
       return Error(Tok.getLoc(), "unknown token in expression");
+    case AsmToken::Error:
+      return Error(getLexer().getErrLoc(), getLexer().getErr());
+      break;
     case AsmToken::EndOfStatement:
       Done = true;
       break;
     case AsmToken::Real:
       // DotOperator: [ebx].0
       UpdateLocLex = false;
       if (ParseIntelDotOperator(SM, End))
         return true;

llvm/test/tools/llvm-ml/radix.test

+# RUN: llvm-ml -filetype=asm %s | FileCheck %s
+
+.code
+
+t1:
+mov eax, 100b
+mov eax, 100y
+
+; CHECK-LABEL: t1:
+; CHECK-NEXT: mov eax, 4
+; CHECK-NEXT: mov eax, 4
+
+t2:
+mov eax, 100o
+mov eax, 100q
+
+; CHECK-LABEL: t2:
+; CHECK-NEXT: mov eax, 64
+; CHECK-NEXT: mov eax, 64
+
+t3:
+mov eax, 100d
+mov eax, 100t
+
+; CHECK-LABEL: t3:
+; CHECK-NEXT: mov eax, 100
+; CHECK-NEXT: mov eax, 100
+
+t4:
+mov eax, 100h
+
+; CHECK-LABEL: t4:
+; CHECK-NEXT: mov eax, 256
+
+t5:
+mov eax, 100
+.radix 2
+mov eax, 100
+.radix 16
+mov eax, 100
+.radix 10
+mov eax, 100
+
+; CHECK-LABEL: t5:
+; CHECK: mov eax, 100
+; CHECK: mov eax, 4
+; CHECK: mov eax, 256
+; CHECK: mov eax, 100
+
+t6:
+.radix 9
+mov eax, 100
+.radix 10
+
+; CHECK-LABEL: t6:
+; CHECK: mov eax, 81
+
+t7:
+.radix 12
+mov eax, 100b
+mov eax, 100y
+.radix 10
+
+; CHECK-LABEL: t7:
+; CHECK: mov eax, 1739
+; CHECK: mov eax, 4
+
+t8:
+.radix 16
+mov eax, 100d
+mov eax, 100t
+.radix 10
+
+; CHECK-LABEL: t8:
+; CHECK: mov eax, 4109
+; CHECK: mov eax, 100
+
+t9:
+.radix 12
+mov eax, 102b
+.radix 16
+mov eax, 10fd
+.radix 10
+
+; CHECK-LABEL: t9:
+; CHECK: mov eax, 1763
+; CHECK: mov eax, 4349
+
+t10:
+.radix 16
+mov eax, 1e1
+.radix 10
+
+; CHECK-LABEL: t10:
+; CHECK: mov eax, 481
+
+END

llvm/test/tools/llvm-ml/radix_errors.test

+; RUN: not llvm-ml -filetype=asm %s 2>&1 | FileCheck %s
+
+.code
+
+t1:
+mov eax, 120b
+mov eax, 120y
+.radix 11
+mov eax, 120b
+mov eax, 120y
+.radix 10
+
+; CHECK: error: invalid decimal number
+; CHECK: error: invalid binary number
+; CHECK: error: invalid base-11 number
+; CHECK: error: invalid binary number
+
+t2:
+mov eax, 190o
+mov eax, 190q
+.radix 13
+mov eax, 190o
+mov eax, 190q
+.radix 10
+
+; CHECK: error: invalid octal number
+; CHECK: error: invalid octal number
+; CHECK: error: invalid octal number
+; CHECK: error: invalid octal number
+
+t3:
+mov eax, 1f0d
+mov eax, 1f0t
+.radix 13
+mov eax, 1f0d
+mov eax, 1f0t
+.radix 10
+
+; CHECK: error: invalid decimal number
+; CHECK: error: invalid decimal number
+; CHECK: error: invalid base-13 number
+; CHECK: error: invalid decimal number
+
+t4:
+mov eax, 10e
+.radix 16
+.radix 10
+mov eax, 10e
+
+; CHECK: error: invalid decimal number
+; CHECK: error: invalid decimal number
+
+t5:
+.radix 9
+mov eax, 9
+.radix 10
+
+; CHECK: error: invalid base-9 number
+
+END