[MC] Separate masm integer literal lexer support from inline asm

[rnk]

Summary

This effectively renames the IsParsingMSInlineAsm member variable of
AsmLexer to LexMasmIntegers and moves it up to MCAsmLexer. This is the
only behavior controlled by that variable. I added a public setter, so
that it can be set from outside or from the command line.

Now, masm integers (0b1101 and 0ABCh) work in __asm blocks from clang,
but 0b label references work when using .intel_syntax in standalone .s
files.

However, 0b label references will *not* work from __asm blocks in clang.
They will work from GCC inline asm blocks, which it sounds like is
important for Crypto++ as mentioned in PR36144.

Essentially, we only lex masm literals when the asm blob itself is
marked as using intel syntax. If the user emits .intel_syntax directly
in the asm string, masm literals will not be lexed, as is the case if we
did a standalone .s compilation.

Hopefully this addresses PR36144.

[mstorsjo]

FWIW, a different bug report where normal GNU non-inline assembly is broken by the MS inline asm handling is PR32973. It doesn't seem to get fixed by this change unfortunately - I haven't dug any further to see how closely related the issues are; that's also a case where e.g. 8h gets treated as a hex literal and gets replaced by 8 where it definitely shouldn't - where no intel syntax is involved whatsoever.

[rnk]

• fix mstorsjo's bug too

[rnk]

• add test case

[rnk]

In D53535#1271972, @mstorsjo wrote:

FWIW, a different bug report where normal GNU non-inline assembly is broken by the MS inline asm handling is PR32973. It doesn't seem to get fixed by this change unfortunately - I haven't dug any further to see how closely related the issues are; that's also a case where e.g. 8h gets treated as a hex literal and gets replaced by 8 where it definitely shouldn't - where no intel syntax is involved whatsoever.

I looked at the code, and there was extra 'h' hex literal handling that wasn't conditionalized the way it was supposed to be. Hopefully this new patch addresses it.

[mstorsjo]

In D53535#1273593, @rnk wrote:

In D53535#1271972, @mstorsjo wrote:

FWIW, a different bug report where normal GNU non-inline assembly is broken by the MS inline asm handling is PR32973. It doesn't seem to get fixed by this change unfortunately - I haven't dug any further to see how closely related the issues are; that's also a case where e.g. 8h gets treated as a hex literal and gets replaced by 8 where it definitely shouldn't - where no intel syntax is involved whatsoever.

I looked at the code, and there was extra 'h' hex literal handling that wasn't conditionalized the way it was supposed to be. Hopefully this new patch addresses it.

Nice! Indeed, it does seem to be fixed by this change now. Thanks!

[thegameg]

LGTM, thanks for looking into this Reid!

llvm/trunk/include/llvm/MC/MCParser/AsmLexer.h

 /// AsmLexer - Lexer class for assembly files.
 class AsmLexer : public MCAsmLexer {
   const MCAsmInfo &MAI;
 
   const char *CurPtr = nullptr;
   StringRef CurBuf;
   bool IsAtStartOfLine = true;
   bool IsAtStartOfStatement = true;
-  bool IsParsingMSInlineAsm = false;
   bool IsPeeking = false;
 
 protected:
   /// LexToken - Read the next token and return its code.
   AsmToken LexToken() override;
 
 public:
   AsmLexer(const MCAsmInfo &MAI);
   AsmLexer(const AsmLexer &) = delete;
   AsmLexer &operator=(const AsmLexer &) = delete;
   ~AsmLexer() override;
 
   void setBuffer(StringRef Buf, const char *ptr = nullptr);
-  void setParsingMSInlineAsm(bool V) { IsParsingMSInlineAsm = V; }
 
   StringRef LexUntilEndOfStatement() override;
 
   size_t peekTokens(MutableArrayRef<AsmToken> Buf,
                     bool ShouldSkipSpace = true) override;
 
   const MCAsmInfo &getMAI() const { return MAI; }
 

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

class MCAsmLexer {
   SMLoc ErrLoc;
   std::string Err;
 
 protected: // Can only create subclasses.
   const char *TokStart = nullptr;
   bool SkipSpace = true;
   bool AllowAtInIdentifier;
   bool IsAtStartOfStatement = true;
+  bool LexMasmIntegers = false;
   AsmCommentConsumer *CommentConsumer = nullptr;
 
   MCAsmLexer();
 
   virtual AsmToken LexToken() = 0;
 
   void SetError(SMLoc errLoc, const std::string &err) {
     ErrLoc = errLoc;
public:
   void setSkipSpace(bool val) { SkipSpace = val; }
 
   bool getAllowAtInIdentifier() { return AllowAtInIdentifier; }
   void setAllowAtInIdentifier(bool v) { AllowAtInIdentifier = v; }
 
   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/trunk/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

void AsmPrinter::EmitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
   std::unique_ptr<MCTargetAsmParser> TAP(TM.getTarget().createMCAsmParser(
       STI, *Parser, *MII, MCOptions));
   if (!TAP)
     report_fatal_error("Inline asm not supported by this streamer because"
                        " we don't have an asm parser for this target\n");
   Parser->setAssemblerDialect(Dialect);
   Parser->setTargetParser(*TAP.get());
   Parser->setEnablePrintSchedInfo(EnablePrintSchedInfo);
+  // Enable lexing Masm binary and hex integer literals in intel inline
+  // assembly.
   if (Dialect == InlineAsm::AD_Intel)
-    // We need this flag to be able to parse numbers like "0bH"
-    Parser->setParsingInlineAsm(true);
+    Parser->getLexer().setLexMasmIntegers(true);
   if (MF) {
     const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
     TAP->SetFrameRegister(TRI->getFrameRegister(*MF));
   }
 
   emitInlineAsmStart();
   // Don't implicitly switch to the text section before the asm.
   int Res = Parser->Run(/*NoInitialTextSection*/ true,

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

static void SkipIgnoredIntegerSuffix(const char *&CurPtr) {
   if (CurPtr[0] == 'L')
     ++CurPtr;
   if (CurPtr[0] == 'L')
     ++CurPtr;
 }
 
 // Look ahead to search for first non-hex digit, if it's [hH], then we treat the
 // integer as a hexadecimal, possibly with leading zeroes.
-static unsigned doLookAhead(const char *&CurPtr, unsigned DefaultRadix) {
-  const char *FirstHex = nullptr;
+static unsigned doHexLookAhead(const char *&CurPtr, unsigned DefaultRadix,
+                               bool LexHex) {
+  const char *FirstNonDec = nullptr;
   const char *LookAhead = CurPtr;
   while (true) {
     if (isDigit(*LookAhead)) {
       ++LookAhead;
-    } else if (isHexDigit(*LookAhead)) {
-      if (!FirstHex)
-        FirstHex = LookAhead;
-      ++LookAhead;
     } else {
+      if (!FirstNonDec)
+        FirstNonDec = LookAhead;
+
+      // Keep going if we are looking for a 'h' suffix.
+      if (LexHex && isHexDigit(*LookAhead))
+        ++LookAhead;
+      else
         break;
     }
   }
-  bool isHex = *LookAhead == 'h' || *LookAhead == 'H';
-  CurPtr = isHex || !FirstHex ? LookAhead : FirstHex;
+  bool isHex = LexHex && (*LookAhead == 'h' || *LookAhead == 'H');
+  CurPtr = isHex || !FirstNonDec ? LookAhead : FirstNonDec;
   if (isHex)
     return 16;
   return DefaultRadix;
 }
 
 static AsmToken intToken(StringRef Ref, APInt &Value)
 {
   if (Value.isIntN(64))
     return AsmToken(AsmToken::Integer, Ref, Value);
   return AsmToken(AsmToken::BigNum, Ref, Value);
 }
 
 /// 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 hexadecimal integer: [0-9][0-9a-fA-F]*[hH]
-  if (IsParsingMSInlineAsm && isdigit(CurPtr[-1])) {
+  if (LexMasmIntegers && isdigit(CurPtr[-1])) {
     const char *FirstNonBinary = (CurPtr[-1] != '0' && CurPtr[-1] != '1') ?
                                    CurPtr - 1 : nullptr;
     const char *OldCurPtr = CurPtr;
     while (isHexDigit(*CurPtr)) {
       if (*CurPtr != '0' && *CurPtr != '1' && !FirstNonBinary)
         FirstNonBinary = CurPtr;
       ++CurPtr;
     }
AsmToken AsmLexer::LexDigit() {
    }
 
     // octal/decimal integers, or floating point numbers, fall through
     CurPtr = OldCurPtr;
   }
 
   // Decimal integer: [1-9][0-9]*
   if (CurPtr[-1] != '0' || CurPtr[0] == '.') {
-    unsigned Radix = doLookAhead(CurPtr, 10);
+    unsigned Radix = doHexLookAhead(CurPtr, 10, LexMasmIntegers);
     bool isHex = Radix == 16;
     // Check for floating point literals.
     if (!isHex && (*CurPtr == '.' || *CurPtr == 'e')) {
       ++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 [bB][hH].
-    if (Radix == 2 || Radix == 16)
+    // Consume the [hH].
+    if (LexMasmIntegers && Radix == 16)
       ++CurPtr;
 
     // The darwin/x86 (and x86-64) assembler accepts and ignores type
     // suffices on integer literals.
     SkipIgnoredIntegerSuffix(CurPtr);
 
     return intToken(Result, Value);
   }
 
-  if (!IsParsingMSInlineAsm && ((*CurPtr == 'b') || (*CurPtr == 'B'))) {
+  if (!LexMasmIntegers && ((*CurPtr == 'b') || (*CurPtr == 'B'))) {
     ++CurPtr;
     // See if we actually have "0b" as part of something like "jmp 0b\n"
     if (!isDigit(CurPtr[0])) {
       --CurPtr;
       StringRef Result(TokStart, CurPtr - TokStart);
       return AsmToken(AsmToken::Integer, Result, 0);
     }
     const char *NumStart = CurPtr;
if ((*CurPtr == 'x') || (*CurPtr == 'X')) {
     if (CurPtr == NumStart)
       return ReturnError(CurPtr-2, "invalid hexadecimal number");
 
     APInt Result(128, 0);
     if (StringRef(TokStart, CurPtr - TokStart).getAsInteger(0, Result))
       return ReturnError(TokStart, "invalid hexadecimal number");
 
     // Consume the optional [hH].
-    if (!IsParsingMSInlineAsm && (*CurPtr == 'h' || *CurPtr == 'H'))
+    if (LexMasmIntegers && (*CurPtr == 'h' || *CurPtr == 'H'))
       ++CurPtr;
 
     // The darwin/x86 (and x86-64) assembler accepts and ignores ULL and LL
     // suffixes on integer literals.
     SkipIgnoredIntegerSuffix(CurPtr);
 
     return intToken(StringRef(TokStart, CurPtr - TokStart), Result);
   }
 
   // Either octal or hexadecimal.
   APInt Value(128, 0, true);
-  unsigned Radix = doLookAhead(CurPtr, 8);
+  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");
 
   // Consume the [hH].
   if (Radix == 16)

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

public:
   void Note(SMLoc L, const Twine &Msg, SMRange Range = None) override;
   bool Warning(SMLoc L, const Twine &Msg, SMRange Range = None) override;
   bool printError(SMLoc L, const Twine &Msg, SMRange Range = None) override;
 
   const AsmToken &Lex() override;
 
   void setParsingInlineAsm(bool V) override {
     ParsingInlineAsm = V;
-    Lexer.setParsingMSInlineAsm(V);
+    // When parsing MS inline asm, we must lex 0b1101 and 0ABCH as binary and
+    // hex integer literals.
+    Lexer.setLexMasmIntegers(V);
   }
   bool isParsingInlineAsm() override { return ParsingInlineAsm; }
 
   bool parseMSInlineAsm(void *AsmLoc, std::string &AsmString,
                         unsigned &NumOutputs, unsigned &NumInputs,
                         SmallVectorImpl<std::pair<void *,bool>> &OpDecls,
                         SmallVectorImpl<std::string> &Constraints,
                         SmallVectorImpl<std::string> &Clobbers,

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

 }
 
 bool X86AsmParser::ParseDirective(AsmToken DirectiveID) {
   MCAsmParser &Parser = getParser();
   StringRef IDVal = DirectiveID.getIdentifier();
   if (IDVal.startswith(".code"))
     return ParseDirectiveCode(IDVal, DirectiveID.getLoc());
   else if (IDVal.startswith(".att_syntax")) {
-    getParser().setParsingInlineAsm(false);
     if (getLexer().isNot(AsmToken::EndOfStatement)) {
       if (Parser.getTok().getString() == "prefix")
         Parser.Lex();
       else if (Parser.getTok().getString() == "noprefix")
         return Error(DirectiveID.getLoc(), "'.att_syntax noprefix' is not "
                                            "supported: registers must have a "
                                            "'%' prefix in .att_syntax");
     }
     getParser().setAssemblerDialect(0);
     return false;
   } else if (IDVal.startswith(".intel_syntax")) {
     getParser().setAssemblerDialect(1);
-    getParser().setParsingInlineAsm(true);
     if (getLexer().isNot(AsmToken::EndOfStatement)) {
       if (Parser.getTok().getString() == "noprefix")
         Parser.Lex();
       else if (Parser.getTok().getString() == "prefix")
         return Error(DirectiveID.getLoc(), "'.intel_syntax prefix' is not "
                                            "supported: registers must not have "
                                            "a '%' prefix in .intel_syntax");
     }

llvm/trunk/test/MC/AArch64/macro-hex-int.s

+// RUN: llvm-mc -triple aarch64-elf -filetype=obj %s -o - | llvm-objdump -d -r - | FileCheck %s
+
+.macro do_add sz
+        add     v0.\sz, v0.\sz, v0.\sz
+.endm
+
+do_add 8h
+// CHECK:  add     v0.8h, v0.8h, v0.8h

llvm/trunk/test/MC/X86/intel-syntax-hex.s

-// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+// RUN: llvm-mc -masm-integers -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
 // rdar://12470373
 
 // Checks to make sure we parse the hexadecimal suffix properly.
 // CHECK: movl $10, %eax
   mov eax, 10
 // CHECK: movl $16, %eax
   mov eax, 10h
 // CHECK: movl $16, %eax

llvm/trunk/test/MC/X86/pr27884.s

-// RUN: llvm-mc -triple x86_64-unknown-unknown %s
+// RUN: llvm-mc -triple x86_64-unknown-unknown %s -masm-integers=1
 
 .intel_syntax
 add rbx, 0B0h
 add rbx, 0b0h
 add rax, 0A0h
 add rax, 0a0h

llvm/trunk/test/tools/llvm-mca/X86/intel-syntax.s

 # NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
 # RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -resource-pressure=false                       < %s | FileCheck %s -check-prefixes=ALL,INTEL
 # RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -resource-pressure=false -output-asm-variant=0 < %s | FileCheck %s -check-prefixes=ALL,ATT
 # RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -resource-pressure=false -output-asm-variant=1 < %s | FileCheck %s -check-prefixes=ALL,INTEL
 
   .intel_syntax noprefix
   mov eax, 1
-  mov ebx, 0ffh
+  mov ebx, 0xff
   imul  esi, edi
   lea eax, [rsi + rdi]
 
 # ALL:        Iterations:        100
 # ALL-NEXT:   Instructions:      400
 # ALL-NEXT:   Total Cycles:      305
 # ALL-NEXT:   Total uOps:        500
 

llvm/trunk/tools/llvm-mc/llvm-mc.cpp

 
 static cl::opt<std::string>
 MainFileName("main-file-name",
              cl::desc("Specifies the name we should consider the input file"));
 
 static cl::opt<bool> SaveTempLabels("save-temp-labels",
                                     cl::desc("Don't discard temporary labels"));
 
+static cl::opt<bool> LexMasmIntegers(
+    "masm-integers",
+    cl::desc("Enable binary and hex masm integers (0b110 and 0ABCh)"));
+
 static cl::opt<bool> NoExecStack("no-exec-stack",
                                  cl::desc("File doesn't need an exec stack"));
 
 enum ActionType {
   AC_AsLex,
   AC_Assemble,
   AC_Disassemble,
   AC_MDisassemble,
if (!TAP) {
     return 1;
   }
 
   int SymbolResult = fillCommandLineSymbols(*Parser);
   if(SymbolResult)
     return SymbolResult;
   Parser->setShowParsedOperands(ShowInstOperands);
   Parser->setTargetParser(*TAP);
+  Parser->getLexer().setLexMasmIntegers(LexMasmIntegers);
 
   int Res = Parser->Run(NoInitialTextSection);
 
   return Res;
 }
 
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);