[MC] AsmLexer: add extensible identifier's character set support.

[vpykhtin]

Summary

Working on AMDGPU project I need to support assembler identifiers started with '&'. Looking into AsmLexer.cpp I found similar need to optionally support '@' inside identifiers and decided this is time to add generic support for identifier charset. I added configurable bitvector set for prefix and body identifier's characters.

[vpykhtin]

Kind reminder if someone can take a look at this.

[• rafael]

include/llvm/MC/MCParser/MCAsmLexer.h

211

Why do you need to make these virtual?

216

is..Contains is a strange name since it has two verbs.

lib/MC/MCParser/AsmLexer.cpp

23

Why?

[vpykhtin]

include/llvm/MC/MCParser/MCAsmLexer.h

211

Well not making it virtual would require bitvector sets to be part of this class. I'm not objecting though as it already done with SkipSpace and AllowAtInIdentifier.

216

What would be a better name here?

lib/MC/MCParser/AsmLexer.cpp

23

Well it based on my previuos experience on Windows where we had lexer using these routines eating up to 10% of scan time. Probably not so "generally" as I stated though. I'm not insisting on this particular change and can remove it.

[vpykhtin]

After a loooong time I would like to reanimate this review requiest.

Previously I incorrectly measured performance impact for this patch and obtained 30% performance gain - this result was incorrect. Current measurement on a large .s file shows no affect on parsing performance.

[vpykhtin]

ping

[vpykhtin]

ping.

[vpykhtin]

Last ping?

[grosbach]

include/llvm/MC/MCParser/MCAsmLexer.h

211

With the generalization, these can go away entirely, yes? Replace the callsites w/ the new API.

219

This should start with "is" not "Is" according the the coding guidelines.

224

Ditto.

231

This feels really weird. Wouldn't any callsites want to be using one of the other two? They'll know their context. I don't see any invocations of this method in the patch. Why is it needed at all?

lib/MC/MCParser/AsmLexer.cpp

568

Can you elaborate on this bit? Not sure I follow why this is so much more logic than previously.

lib/MC/MCParser/MCAsmLexer.cpp

24

Given the bimodal behaviour based on Value, this should probably just be two functions.

include/llvm/MC/MCParser/AsmLexer.h

 //===- AsmLexer.h - Lexer for Assembly Files --------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This class declares the lexer for assembly files.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_MC_MCPARSER_ASMLEXER_H
 #define LLVM_MC_MCPARSER_ASMLEXER_H
 
+#include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/MC/MCParser/MCAsmLexer.h"
 #include "llvm/Support/DataTypes.h"
 #include <string>
 
 namespace llvm {
 class MemoryBuffer;
 class MCAsmInfo;
 
 /// AsmLexer - Lexer class for assembly files.
 class AsmLexer : public MCAsmLexer {
   const MCAsmInfo &MAI;
 
   const char *CurPtr;
   StringRef CurBuf;
   bool isAtStartOfLine;
 
+  SmallBitVector IdPrefixCharSet;
+  SmallBitVector IdBodyCharSet;
+
   void operator=(const AsmLexer&) = delete;
   AsmLexer(const AsmLexer&) = delete;
 
 protected:
   /// LexToken - Read the next token and return its code.
   AsmToken LexToken() override;
 
 public:
public:
   bool isAtStatementSeparator(const char *Ptr);
 
   const MCAsmInfo &getMAI() const { return MAI; }
 
 private:
   int getNextChar();
   AsmToken ReturnError(const char *Loc, const std::string &Msg);
 
+  bool IsIdentifierPrefixChar(char c) const;
+  bool IsIdentifierBodyChar(char c) const;
+
+  void setIdentifierCharSet(bool Value,
+                            StringRef PfxCharSet,
+                            StringRef BodyCharSet) override;
+  bool isIdentifierCharSetContains(char) const override;
+
   AsmToken LexIdentifier();
   AsmToken LexSlash();
   AsmToken LexLineComment();
   AsmToken LexDigit();
   AsmToken LexSingleQuote();
   AsmToken LexQuote();
   AsmToken LexFloatLiteral();
   AsmToken LexHexFloatLiteral(bool NoIntDigits);
 };
 
 } // end namespace llvm
 
 #endif

include/llvm/MC/MCParser/MCAsmLexer.h

class MCAsmLexer {
   SMLoc ErrLoc;
   std::string Err;
 
   MCAsmLexer(const MCAsmLexer &) = delete;
   void operator=(const MCAsmLexer &) = delete;
 protected: // Can only create subclasses.
   const char *TokStart;
   bool SkipSpace;
-  bool AllowAtInIdentifier;
 
   MCAsmLexer();
 
   virtual AsmToken LexToken() = 0;
 
   void SetError(SMLoc errLoc, const std::string &err) {
     ErrLoc = errLoc;
     Err = err;
public:
   bool is(AsmToken::TokenKind K) const { return getTok().is(K); }
 
   /// Check if the current token has kind \p K.
   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; }
+  /// allow/disallow an identifier to contain specified characters
+  virtual void setIdentifierCharSet(bool Value,
+                                    StringRef PfxCharSet,
+                                    StringRef BodyCharSet);
+
+  /// test whether the specified character can be found in an identifier
+  virtual bool isIdentifierCharSetContains(char) const = 0;
+
+  bool getAllowAtInIdentifier() { return isIdentifierCharSetContains('@'); }
+  void setAllowAtInIdentifier(bool v) { setIdentifierCharSet(v, "", "@"); }
 };
 
 } // End llvm namespace
 
 #endif

lib/MC/MCParser/AsmLexer.cpp

 // This class implements the lexer for assembly files.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/MC/MCParser/AsmLexer.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SMLoc.h"
-#include <cctype>
 #include <cerrno>
 #include <cstdio>
 #include <cstdlib>
 using namespace llvm;
 
+// standard is(x)digit generally much slower than simple
+// checks like below
+inline static bool isDigit(char C) {
+  return (C >= '0' && C <= '9');
+}
+
+inline static bool isHexDigit(char C) {
+  return isDigit(C)
+         || (C >= 'A' && C <= 'F')
+         || (C >= 'a' && C <= 'f');
+}
+
 AsmLexer::AsmLexer(const MCAsmInfo &MAI) : MAI(MAI) {
   CurPtr = nullptr;
   isAtStartOfLine = true;
-  AllowAtInIdentifier = !StringRef(MAI.getCommentString()).startswith("@");
+
+  IdPrefixCharSet.resize(256);
+  IdPrefixCharSet.set('a', 'z' + 1);
+  IdPrefixCharSet.set('A', 'Z' + 1);
+  IdPrefixCharSet.set('.');
+  IdPrefixCharSet.set('_');
+
+  IdBodyCharSet = IdPrefixCharSet;
+  IdBodyCharSet.set('0', '9' + 1);
+  IdBodyCharSet.set('$');
+  IdBodyCharSet.set('?');
+
+  if (!StringRef(MAI.getCommentString()).startswith("@"))
+    IdBodyCharSet.set('@');
 }
 
 AsmLexer::~AsmLexer() {
 }
 
+void AsmLexer::setIdentifierCharSet(bool Value,
+                                    StringRef PfxCharSet,
+                                    StringRef BodyCharSet) {
+  if (Value) {
+    for (auto C : PfxCharSet)
+      IdPrefixCharSet.set((unsigned char)C);
+    for (auto C : BodyCharSet)
+      IdBodyCharSet.set((unsigned char)C);
+  } else {
+    for (auto C : PfxCharSet)
+      IdPrefixCharSet.reset((unsigned char)C);
+    for (auto C : BodyCharSet)
+      IdBodyCharSet.reset((unsigned char)C);
+  }
+}
+
+inline bool AsmLexer::IsIdentifierPrefixChar(char C) const {
+  return IdPrefixCharSet.test((unsigned char)C);
+}
+
+inline bool AsmLexer::IsIdentifierBodyChar(char C) const {
+  return IdBodyCharSet.test((unsigned char)C);
+}
+
+bool AsmLexer::isIdentifierCharSetContains(char C) const {
+  return IsIdentifierBodyChar(C) || IsIdentifierPrefixChar(C);
+}
+
 void AsmLexer::setBuffer(StringRef Buf, const char *ptr) {
   CurBuf = Buf;
 
   if (ptr)
     CurPtr = ptr;
   else
     CurPtr = CurBuf.begin();
 

 
 /// LexFloatLiteral: [0-9]*[.][0-9]*([eE][+-]?[0-9]*)?
 ///
 /// The leading integral digit sequence and dot should have already been
 /// consumed, some or all of the fractional digit sequence *can* have been
 /// consumed.
 AsmToken AsmLexer::LexFloatLiteral() {
   // Skip the fractional digit sequence.
-  while (isdigit(*CurPtr))
+  while (isDigit(*CurPtr))
     ++CurPtr;
 
   // Check for exponent; we intentionally accept a slighlty wider set of
   // literals here and rely on the upstream client to reject invalid ones (e.g.,
   // "1e+").
   if (*CurPtr == 'e' || *CurPtr == 'E') {
     ++CurPtr;
     if (*CurPtr == '-' || *CurPtr == '+')
       ++CurPtr;
-    while (isdigit(*CurPtr))
+    while (isDigit(*CurPtr))
       ++CurPtr;
   }
 
   return AsmToken(AsmToken::Real,
                   StringRef(TokStart, CurPtr - TokStart));
 }
 
 /// LexHexFloatLiteral matches essentially (.[0-9a-fA-F]*)?[pP][+-]?[0-9a-fA-F]+
 /// while making sure there are enough actual digits around for the constant to
 /// be valid.
 ///
 /// The leading "0x[0-9a-fA-F]*" (i.e. integer part) has already been consumed
 /// before we get here.
 AsmToken AsmLexer::LexHexFloatLiteral(bool NoIntDigits) {
   assert((*CurPtr == 'p' || *CurPtr == 'P' || *CurPtr == '.') &&
          "unexpected parse state in floating hex");
   bool NoFracDigits = true;
 
   // Skip the fractional part if there is one
   if (*CurPtr == '.') {
     ++CurPtr;
 
     const char *FracStart = CurPtr;
-    while (isxdigit(*CurPtr))
+    while (isHexDigit(*CurPtr))
       ++CurPtr;
 
     NoFracDigits = CurPtr == FracStart;
   }
 
   if (NoIntDigits && NoFracDigits)
     return ReturnError(TokStart, "invalid hexadecimal floating-point constant: "
                                  "expected at least one significand digit");
 
   // Make sure we do have some kind of proper exponent part
   if (*CurPtr != 'p' && *CurPtr != 'P')
     return ReturnError(TokStart, "invalid hexadecimal floating-point constant: "
                                  "expected exponent part 'p'");
   ++CurPtr;
 
   if (*CurPtr == '+' || *CurPtr == '-')
     ++CurPtr;
 
   // N.b. exponent digits are *not* hex
   const char *ExpStart = CurPtr;
-  while (isdigit(*CurPtr))
+  while (isDigit(*CurPtr))
     ++CurPtr;
 
   if (CurPtr == ExpStart)
     return ReturnError(TokStart, "invalid hexadecimal floating-point constant: "
                                  "expected at least one exponent digit");
 
   return AsmToken(AsmToken::Real, StringRef(TokStart, CurPtr - TokStart));
 }
 
-/// LexIdentifier: [a-zA-Z_.][a-zA-Z0-9_$.@?]*
-static bool IsIdentifierChar(char c, bool AllowAt) {
-  return isalnum(c) || c == '_' || c == '$' || c == '.' ||
-         (c == '@' && AllowAt) || c == '?';
-}
 AsmToken AsmLexer::LexIdentifier() {
-  // Check for floating point literals.
-  if (CurPtr[-1] == '.' && isdigit(*CurPtr)) {
-    // Disambiguate a .1243foo identifier from a floating literal.
-    while (isdigit(*CurPtr))
-      ++CurPtr;
-    if (*CurPtr == 'e' || *CurPtr == 'E' ||
-        !IsIdentifierChar(*CurPtr, AllowAtInIdentifier))
-      return LexFloatLiteral();
-  }
-
-  while (IsIdentifierChar(*CurPtr, AllowAtInIdentifier))
+  assert(IsIdentifierPrefixChar(*TokStart));
+  while (IsIdentifierBodyChar(*CurPtr))
     ++CurPtr;
-
-  // Handle . as a special case.
-  if (CurPtr == TokStart+1 && TokStart[0] == '.')
-    return AsmToken(AsmToken::Dot, StringRef(TokStart, 1));
-
   return AsmToken(AsmToken::Identifier, StringRef(TokStart, CurPtr - TokStart));
 }
 
 /// LexSlash: Slash: /
 ///           C-Style Comment: /* ... */
 AsmToken AsmLexer::LexSlash() {
   switch (*CurPtr) {
   case '*': break; // C style comment.

 }
 
 // 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;
   const char *LookAhead = CurPtr;
   while (1) {
-    if (isdigit(*LookAhead)) {
+    if (isDigit(*LookAhead)) {
       ++LookAhead;
-    } else if (isxdigit(*LookAhead)) {
+    } else if (isHexDigit(*LookAhead)) {
       if (!FirstHex)
         FirstHex = LookAhead;
       ++LookAhead;
     } else {
       break;
     }
   }
   bool isHex = *LookAhead == 'h' || *LookAhead == 'H';
if (CurPtr[-1] != '0' || CurPtr[0] == '.') {
     SkipIgnoredIntegerSuffix(CurPtr);
 
     return intToken(Result, Value);
   }
 
   if (*CurPtr == 'b') {
     ++CurPtr;
     // See if we actually have "0b" as part of something like "jmp 0b\n"
-    if (!isdigit(CurPtr[0])) {
+    if (!isDigit(CurPtr[0])) {
       --CurPtr;
       StringRef Result(TokStart, CurPtr - TokStart);
       return AsmToken(AsmToken::Integer, Result, 0);
     }
     const char *NumStart = CurPtr;
     while (CurPtr[0] == '0' || CurPtr[0] == '1')
       ++CurPtr;
 
if (*CurPtr == 'b') {
     SkipIgnoredIntegerSuffix(CurPtr);
 
     return intToken(Result, Value);
   }
 
   if (*CurPtr == 'x') {
     ++CurPtr;
     const char *NumStart = CurPtr;
-    while (isxdigit(CurPtr[0]))
+    while (isHexDigit(CurPtr[0]))
       ++CurPtr;
 
     // "0x.0p0" is valid, and "0x0p0" (but not "0xp0" for example, which will be
     // diagnosed by LexHexFloatLiteral).
     if (CurPtr[0] == '.' || CurPtr[0] == 'p' || CurPtr[0] == 'P')
       return LexHexFloatLiteral(NumStart == CurPtr);
 
     // Otherwise requires at least one hex digit.

 bool AsmLexer::isAtStatementSeparator(const char *Ptr) {
   return strncmp(Ptr, MAI.getSeparatorString(),
                  strlen(MAI.getSeparatorString())) == 0;
 }
 
 AsmToken AsmLexer::LexToken() {
   TokStart = CurPtr;
   // This always consumes at least one character.
-  int CurChar = getNextChar();
+  const int CurChar = getNextChar();
 
   if (isAtStartOfComment(TokStart)) {
     // If this comment starts with a '#', then return the Hash token and let
     // the assembler parser see if it can be parsed as a cpp line filename
     // comment. We do this only if we are at the start of a line.
     if (CurChar == '#' && isAtStartOfLine)
       return AsmToken(AsmToken::Hash, StringRef(TokStart, 1));
     isAtStartOfLine = true;
     return LexLineComment();
   }
   if (isAtStatementSeparator(TokStart)) {
     CurPtr += strlen(MAI.getSeparatorString()) - 1;
     return AsmToken(AsmToken::EndOfStatement,
                     StringRef(TokStart, strlen(MAI.getSeparatorString())));
   }
 
   // If we're missing a newline at EOF, make sure we still get an
   // EndOfStatement token before the Eof token.
   if (CurChar == EOF && !isAtStartOfLine) {
     isAtStartOfLine = true;
     return AsmToken(AsmToken::EndOfStatement, StringRef(TokStart, 1));
   }
 
   isAtStartOfLine = false;
+
+  if (CurChar == '.' && isDigit(*CurPtr)) {
+    if (!IsIdentifierPrefixChar('.'))
+      return LexFloatLiteral();
+
+    const auto SavePos = CurPtr;
+    // Disambiguate a .1243foo identifier from a floating literal.
+    do { ++CurPtr; }
+    while (isDigit(*CurPtr));
+    if (*CurPtr == 'e' || *CurPtr == 'E' || !IsIdentifierBodyChar(*CurPtr))
+      return LexFloatLiteral();
+    CurPtr = SavePos;
+  }
+
+  const AsmToken Id = IsIdentifierPrefixChar(CurChar) ?
+                        LexIdentifier() :
+                        AsmToken(AsmToken::Error, StringRef());
+
+  // if single char id - need further check
+  if (Id.is(AsmToken::Identifier) && Id.getString().size() > 1)
+    return Id;
+
   switch (CurChar) {
   default:
-    // Handle identifier: [a-zA-Z_.][a-zA-Z0-9_$.@]*
-    if (isalpha(CurChar) || CurChar == '_' || CurChar == '.')
-      return LexIdentifier();
+    if (Id.is(AsmToken::Identifier)) // single char id indeed
+      return Id;
 
     // Unknown character, emit an error.
     return ReturnError(TokStart, "invalid character in input");
   case EOF: return AsmToken(AsmToken::Eof, StringRef(TokStart, 0));
   case 0:
   case ' ':
   case '\t':
     if (SkipSpace) {
       // Ignore whitespace.
       return LexToken();
     } else {
       int len = 1;
       while (*CurPtr==' ' || *CurPtr=='\t') {
         CurPtr++;
         len++;
       }
       return AsmToken(AsmToken::Space, StringRef(TokStart, len));
     }
   case '\n': // FALL THROUGH.
   case '\r':
     isAtStartOfLine = true;
     return AsmToken(AsmToken::EndOfStatement, StringRef(TokStart, 1));
+  case '.': return AsmToken(AsmToken::Dot, StringRef(TokStart, 1));
   case ':': return AsmToken(AsmToken::Colon, StringRef(TokStart, 1));
   case '+': return AsmToken(AsmToken::Plus, StringRef(TokStart, 1));
   case '-': return AsmToken(AsmToken::Minus, StringRef(TokStart, 1));
   case '~': return AsmToken(AsmToken::Tilde, StringRef(TokStart, 1));
   case '(': return AsmToken(AsmToken::LParen, StringRef(TokStart, 1));
   case ')': return AsmToken(AsmToken::RParen, StringRef(TokStart, 1));
   case '[': return AsmToken(AsmToken::LBrac, StringRef(TokStart, 1));
   case ']': return AsmToken(AsmToken::RBrac, StringRef(TokStart, 1));

lib/MC/MCParser/MCAsmLexer.cpp

 
 MCAsmLexer::MCAsmLexer() : TokStart(nullptr), SkipSpace(true) {
   CurTok.emplace_back(AsmToken::Error, StringRef());
 }
 
 MCAsmLexer::~MCAsmLexer() {
 }
 
+void MCAsmLexer::setIdentifierCharSet(bool Value,
+                                      StringRef PfxCharSet,
+                                      StringRef BodyCharSet) {
+}
+
 SMLoc MCAsmLexer::getLoc() const {
   return SMLoc::getFromPointer(TokStart);
 }
 
 SMLoc AsmToken::getLoc() const {
   return SMLoc::getFromPointer(Str.data());
 }
 
 SMLoc AsmToken::getEndLoc() const {
   return SMLoc::getFromPointer(Str.data() + Str.size());
 }
 
 SMRange AsmToken::getLocRange() const {
   return SMRange(getLoc(), getEndLoc());
 }