[AsmParser][SystemZ][z/OS] Use updated framework in AsmLexer to accept special tokens as Identifiers

[anirudhp]

Summary

• Previously, https://reviews.llvm.org/D99889 changed the framework in the AsmLexer to treat special tokens, if they occur at the start of the string, as Identifiers.
• These are used by the MASM Parser implementation in LLVM, and we can extend some of the changes made in the previous patch to SystemZ.
• In SystemZ, the special "tokens" referred to here are "_", "$", "@", "#". [_|$|@|#] are already supported as "part" of an Identifier.
• The changes in this patch ensure that these special tokens, when they occur at the start of the Identifier, are treated as Identifiers.

[myiwanch]

Just making sure I understand, this is so an identifier can start with # without being a comment?

[anirudhp]

• Rebase on latest master

[anirudhp]

In D100959#2705833, @myiwanch wrote:

Just making sure I understand, this is so an identifier can start with # without being a comment?

No.
Currently, if the AllowAdditionalComments is set to true, any string starting with # will be lexed as a comment. This is to ensure that if it isn't lexed as a comment, we lex it as an identifier and not as an AsmToken::Hash. Like the comment in MCAsmInfo.h for the added attribute says:

/// If the CommentString is also set to "#", setting this option will have
/// no effect, and the string will be lexed as a comment.

[epastor]

llvm/include/llvm/MC/MCAsmInfo.h

207–208

Is there a place where we could add a test for this behavior, if it's how we want this to behave? Is there some natural environment/configuration where we expect the CommentString to be set to "#" while AllowHashAtStartOfIdentifier is also true?

[anirudhp]

llvm/include/llvm/MC/MCAsmInfo.h

207–208

Sure, I'll update the unit test to test the interaction between the permutations of the two attributes.

By default, CommentString is set to #. But there's another point here. Whenever a comment string starts with # it will be attempted to be lexed as a comment irrespective of what CommentString is set to because the Lexer currently lexes strings that start with # as a possible comment. This is now guarded by the AllowAdditionalComments attribute in MCAsmInfo.h.

So, if AllowHashAtStartOfIdentifier is set to true, then it will be only lexed as an Identifier, if it is not already lexed as a possible comment. If, under the current Lexer's rules, it is possible to be lexed as a comment, it will be done so (and not as an Identifier)

[anirudhp]

• Added additional tests to test behaviour between setting AllowHashAtStartOfIdentifier and when AsmLexer "decides" to lex a string as a possible comment.
• Updated comments for a few attributes in MCAsmInfo.h to make it more clear.
• Rebase on lastest master

llvm/include/llvm/MC/MCAsmInfo.h

207–208

I have added a few more tests.

[abhina.sreeskantharajan]

Thanks for adding tests. LGTM

llvm/include/llvm/MC/MCAsmInfo.h

protected:
   unsigned AssemblerDialect = 0;
 
   /// This is true if the assembler allows @ characters in symbol names.
   /// Defaults to false.
   bool AllowAtInName = false;
 
   /// This is true if the assembler allows the "?" character at the start of
   /// of a string to be lexed as an AsmToken::Identifier.
-  /// If the CommentString is also set to "?", setting this option will have
-  /// no effect, and the string will be lexed as a comment.
-  /// Defaults to false.
+  /// If the AsmLexer determines that the string can be lexed as a possible
+  /// comment, setting this option will have no effect, and the string will
+  /// still be lexed as a comment.
   bool AllowQuestionAtStartOfIdentifier = false;
 
   /// This is true if the assembler allows the "$" character at the start of
   /// of a string to be lexed as an AsmToken::Identifier.
-  /// If the CommentString is also set to "$", setting this option will have
-  /// no effect, and the string will be lexed as a comment.
-  /// Defaults to false.
+  /// If the AsmLexer determines that the string can be lexed as a possible
+  /// comment, setting this option will have no effect, and the string will
+  /// still be lexed as a comment.
   bool AllowDollarAtStartOfIdentifier = false;
 
   /// This is true if the assembler allows the "@" character at the start of
   /// a string to be lexed as an AsmToken::Identifier.
-  /// If the CommentString is also set to "@", setting this option will have
-  /// no effect, and the string will be lexed as a comment.
-  /// Defaults to false.
+  /// If the AsmLexer determines that the string can be lexed as a possible
+  /// comment, setting this option will have no effect, and the string will
+  /// still be lexed as a comment.
   bool AllowAtAtStartOfIdentifier = false;
 
+  /// This is true if the assembler allows the "#" character at the start of
+  /// a string to be lexed as an AsmToken::Identifier.
+  /// If the AsmLexer determines that the string can be lexed as a possible
+  /// comment, setting this option will have no effect, and the string will
+  /// still be lexed as a comment.
+  bool AllowHashAtStartOfIdentifier = false;
+
   /// If this is true, symbol names with invalid characters will be printed in
   /// quotes.
   bool SupportsQuotedNames = true;
 
   /// This is true if data region markers should be printed as
   /// ".data_region/.end_data_region" directives. If false, use "$d/$a" labels
   /// instead.
   bool UseDataRegionDirectives = false;
bool doesAllowQuestionAtStartOfIdentifier() const {
     return AllowQuestionAtStartOfIdentifier;
   }
   bool doesAllowAtAtStartOfIdentifier() const {
     return AllowAtAtStartOfIdentifier;
   }
   bool doesAllowDollarAtStartOfIdentifier() const {
     return AllowDollarAtStartOfIdentifier;
   }
+  bool doesAllowHashAtStartOfIdentifier() const {
+    return AllowHashAtStartOfIdentifier;
+  }
   bool supportsNameQuoting() const { return SupportsQuotedNames; }
 
   bool doesSupportDataRegionDirectives() const {
     return UseDataRegionDirectives;
   }
 
   bool useDotAlignForAlignment() const {
     return UseDotAlignForAlignment;

llvm/lib/MC/MCParser/AsmLexer.cpp

if (MAI.hasMipsExpressions()) {
         CurPtr += OperatorLength - 1;
         return AsmToken(Operator, StringRef(TokStart, OperatorLength));
       }
     }
     return AsmToken(AsmToken::Percent, StringRef(TokStart, 1));
   case '/':
     IsAtStartOfStatement = OldIsAtStartOfStatement;
     return LexSlash();
-  case '#': return AsmToken(AsmToken::Hash, StringRef(TokStart, 1));
+  case '#': {
+    if (MAI.doesAllowHashAtStartOfIdentifier())
+      return LexIdentifier();
+    return AsmToken(AsmToken::Hash, StringRef(TokStart, 1));
+  }
   case '\'': return LexSingleQuote();
   case '"': return LexQuote();
   case '0': case '1': case '2': case '3': case '4':
   case '5': case '6': case '7': case '8': case '9':
     return LexDigit();
   case '<':
     switch (*CurPtr) {
     case '<':

llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCAsmInfo.cpp

SystemZMCAsmInfo::SystemZMCAsmInfo(const Triple &TT) {
 
   AssemblerDialect = TT.isOSzOS() ? AD_HLASM : AD_ATT;
 
   MaxInstLength = 6;
 
   CommentString = AssemblerDialect == AD_HLASM ? "*" : "#";
   RestrictCommentStringToStartOfStatement = (AssemblerDialect == AD_HLASM);
   AllowAdditionalComments = (AssemblerDialect == AD_ATT);
+  AllowAtAtStartOfIdentifier = (AssemblerDialect == AD_HLASM);
+  AllowDollarAtStartOfIdentifier = (AssemblerDialect == AD_HLASM);
+  AllowHashAtStartOfIdentifier = (AssemblerDialect == AD_HLASM);
+
   ZeroDirective = "\t.space\t";
   Data64bitsDirective = "\t.quad\t";
   UsesELFSectionDirectiveForBSS = true;
   SupportsDebugInformation = true;
   ExceptionsType = ExceptionHandling::DwarfCFI;
 }

llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp

void setAllowQuestionAtStartOfIdentifier(bool Value) {
     AllowQuestionAtStartOfIdentifier = Value;
   }
   void setAllowAtAtStartOfIdentifier(bool Value) {
     AllowAtAtStartOfIdentifier = Value;
   }
   void setAllowDollarAtStartOfIdentifier(bool Value) {
     AllowDollarAtStartOfIdentifier = Value;
   }
+  void setAllowHashAtStartOfIdentifier(bool Value) {
+    AllowHashAtStartOfIdentifier = Value;
+  }
 };
 
 // Setup a testing class that the GTest framework can call.
 class SystemZAsmLexerTest : public ::testing::Test {
 protected:
   static void SetUpTestCase() {
     LLVMInitializeSystemZTargetInfo();
     LLVMInitializeSystemZTargetMC();
TEST_F(SystemZAsmLexerTest, CheckAcceptDollarAtStartOfIdentifier) {
 
   // Lex initially to get the string.
   Parser->getLexer().Lex();
 
   SmallVector<AsmToken::TokenKind> ExpectedTokens(
       {AsmToken::Identifier, AsmToken::EndOfStatement, AsmToken::Eof});
   lexAndCheckTokens(AsmStr, ExpectedTokens);
 }
+
+TEST_F(SystemZAsmLexerTest, CheckAcceptHashAtStartOfIdentifier) {
+  StringRef AsmStr = "##a#b$c";
+
+  // Setup.
+  MUPMAI->setAllowHashAtStartOfIdentifier(true);
+  MUPMAI->setCommentString("*");
+  MUPMAI->setAllowAdditionalComments(false);
+  setupCallToAsmParser(AsmStr);
+  Parser->getLexer().setAllowHashInIdentifier(true);
+
+  // Lex initially to get the string.
+  Parser->getLexer().Lex();
+
+  SmallVector<AsmToken::TokenKind> ExpectedTokens(
+      {AsmToken::Identifier, AsmToken::EndOfStatement, AsmToken::Eof});
+  lexAndCheckTokens(AsmStr, ExpectedTokens);
+}
+
+TEST_F(SystemZAsmLexerTest, CheckAcceptHashAtStartOfIdentifier2) {
+  StringRef AsmStr = "##a#b$c";
+
+  // Setup.
+  MUPMAI->setAllowHashAtStartOfIdentifier(true);
+  setupCallToAsmParser(AsmStr);
+  Parser->getLexer().setAllowHashInIdentifier(true);
+
+  // Lex initially to get the string.
+  Parser->getLexer().Lex();
+
+  // By default, the CommentString attribute is set to "#".
+  // Hence, "##a#b$c" is lexed as a line comment irrespective
+  // of whether the AllowHashAtStartOfIdentifier attribute is set to true.
+  SmallVector<AsmToken::TokenKind> ExpectedTokens(
+      {AsmToken::EndOfStatement, AsmToken::Eof});
+  lexAndCheckTokens(AsmStr, ExpectedTokens);
+}
+
+TEST_F(SystemZAsmLexerTest, CheckAcceptHashAtStartOfIdentifier3) {
+  StringRef AsmStr = "##a#b$c";
+
+  // Setup.
+  MUPMAI->setAllowHashAtStartOfIdentifier(true);
+  MUPMAI->setCommentString("*");
+  setupCallToAsmParser(AsmStr);
+  Parser->getLexer().setAllowHashInIdentifier(true);
+
+  // Lex initially to get the string.
+  Parser->getLexer().Lex();
+
+  // By default, the AsmLexer treats strings that start with "#"
+  // as a line comment.
+  // Hence, "##a$b$c" is lexed as a line comment irrespective
+  // of whether the AllowHashAtStartOfIdentifier attribute is set to true.
+  SmallVector<AsmToken::TokenKind> ExpectedTokens(
+      {AsmToken::EndOfStatement, AsmToken::Eof});
+  lexAndCheckTokens(AsmStr, ExpectedTokens);
+}
+
+TEST_F(SystemZAsmLexerTest, CheckAcceptHashAtStartOfIdentifier4) {
+  StringRef AsmStr = "##a#b$c";
+
+  // Setup.
+  MUPMAI->setAllowHashAtStartOfIdentifier(true);
+  MUPMAI->setCommentString("*");
+  MUPMAI->setAllowAdditionalComments(false);
+  setupCallToAsmParser(AsmStr);
+  Parser->getLexer().setAllowHashInIdentifier(true);
+
+  // Lex initially to get the string.
+  Parser->getLexer().Lex();
+
+  // Since, the AllowAdditionalComments attribute is set to false,
+  // only strings starting with the CommentString attribute are
+  // lexed as possible comments.
+  // Hence, "##a$b$c" is lexed as an Identifier because the
+  // AllowHashAtStartOfIdentifier attribute is set to true.
+  SmallVector<AsmToken::TokenKind> ExpectedTokens(
+      {AsmToken::Identifier, AsmToken::EndOfStatement, AsmToken::Eof});
+  lexAndCheckTokens(AsmStr, ExpectedTokens);
+}
 } // end anonymous namespace