[AsmParser][SystemZ][z/OS] Revert - "Introducing HLASM Comment Syntax"

[anirudhp]

Summary

• https://reviews.llvm.org/D97703 causes an issue with builds that are built with BUILD_SHARED_LIBS=ON.
• Reverting for now, until I figure out why it passes with static libs but not shared libs.

[anirudhp]

I haven't reverted a commit posted on Phabricator, but I'm assuming the flow is similar to git. I would appreciate feedback from the reviewers, just to ensure its right. Thanks!

[hubert.reinterpretcast]

Will commit a revert.

[abhina.sreeskantharajan]

LGTM. I think the usual workflow we don't need to create a review for revert commits.

[hubert.reinterpretcast]

Committed: https://github.com/llvm/llvm-project/commit/4f9cc15

llvm/include/llvm/MC/MCAsmInfo.h

protected:
   /// The '$' token, when not referencing an identifier or constant, refers to
   /// the current PC.  Defaults to false.
   bool DollarIsPC = false;
 
   /// This string, if specified, is used to separate instructions from each
   /// other when on the same line.  Defaults to ';'
   const char *SeparatorString;
 
-  /// This indicates the comment string used by the assembler.  Defaults to
+  /// This indicates the comment character used by the assembler.  Defaults to
   /// "#"
   StringRef CommentString;
 
-  /// This indicates whether the comment string is only accepted as a comment
-  /// at the beginning of statements. Defaults to false.
-  bool RestrictCommentStringToStartOfStatement = false;
-
   /// This is appended to emitted labels.  Defaults to ":"
   const char *LabelSuffix;
 
   // Print the EH begin symbol with an assignment. Defaults to false.
   bool UseAssignmentForEHBegin = false;
 
   // Do we need to create a local symbol for .size?
   bool NeedsLocalForSize = false;
public:
   bool getDollarIsPC() const { return DollarIsPC; }
   const char *getSeparatorString() const { return SeparatorString; }
 
   /// This indicates the column (zero-based) at which asm comments should be
   /// printed.
   unsigned getCommentColumn() const { return 40; }
 
   StringRef getCommentString() const { return CommentString; }
-  bool getRestrictCommentStringToStartOfStatement() const {
-    return RestrictCommentStringToStartOfStatement;
-  }
   const char *getLabelSuffix() const { return LabelSuffix; }
 
   bool useAssignmentForEHBegin() const { return UseAssignmentForEHBegin; }
   bool needsLocalForSize() const { return NeedsLocalForSize; }
   StringRef getPrivateGlobalPrefix() const { return PrivateGlobalPrefix; }
   StringRef getPrivateLabelPrefix() const { return PrivateLabelPrefix; }
 
   bool hasLinkerPrivateGlobalPrefix() const {

llvm/lib/MC/MCParser/AsmLexer.cpp

if (Token.is(AsmToken::Eof))
       break;
   }
 
   SetError(SavedErrLoc, SavedErr);
   return ReadCount;
 }
 
 bool AsmLexer::isAtStartOfComment(const char *Ptr) {
-  if (MAI.getRestrictCommentStringToStartOfStatement() && !IsAtStartOfStatement)
-    return false;
-
   StringRef CommentString = MAI.getCommentString();
 
   if (CommentString.size() == 1)
     return CommentString[0] == Ptr[0];
 
   // Allow # preprocessor commments also be counted as comments for "##" cases
   if (CommentString[1] == '#')
     return CommentString[0] == Ptr[0];

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

SystemZMCAsmInfo::SystemZMCAsmInfo(const Triple &TT) {
   CodePointerSize = 8;
   CalleeSaveStackSlotSize = 8;
   IsLittleEndian = false;
 
   AssemblerDialect = TT.isOSzOS() ? AD_HLASM : AD_ATT;
 
   MaxInstLength = 6;
 
-  CommentString = AssemblerDialect == AD_HLASM ? "*" : "#";
-  RestrictCommentStringToStartOfStatement = (AssemblerDialect == AD_HLASM);
+  CommentString = "#";
   ZeroDirective = "\t.space\t";
   Data64bitsDirective = "\t.quad\t";
   UsesELFSectionDirectiveForBSS = true;
   SupportsDebugInformation = true;
   ExceptionsType = ExceptionHandling::DwarfCFI;
 }

llvm/unittests/MC/SystemZ/CMakeLists.txt

-include_directories(
-  ${LLVM_MAIN_SRC_DIR}/lib/Target/SystemZ
-  )
-
-set(LLVM_LINK_COMPONENTS
-  SystemZ
-  MC
-  Support
-  )
-
-add_llvm_unittest(SystemZAsmLexer
-  SystemZAsmLexerTest.cpp
-  )

llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp

-//===- llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp ----------------===//
-//
-// 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
-//
-//===--------------------------------------------------------------------===//
-#include "llvm/MC/MCAsmInfo.h"
-#include "llvm/MC/MCContext.h"
-#include "llvm/MC/MCObjectFileInfo.h"
-#include "llvm/MC/MCParser/MCTargetAsmParser.h"
-#include "llvm/MC/MCRegisterInfo.h"
-#include "llvm/MC/MCStreamer.h"
-#include "llvm/Support/MemoryBuffer.h"
-#include "llvm/Support/SourceMgr.h"
-#include "llvm/Support/TargetRegistry.h"
-#include "llvm/Support/TargetSelect.h"
-
-#include "gtest/gtest.h"
-
-using namespace llvm;
-
-namespace {
-
-// Come up with our hacked version of MCAsmInfo.
-// This hacked version derives from the main MCAsmInfo instance.
-// Here, we're free to override whatever we want, without polluting
-// the main MCAsmInfo interface.
-class MockedUpMCAsmInfo : public MCAsmInfo {
-public:
-  void setRestrictCommentStringToStartOfStatement(bool Value) {
-    RestrictCommentStringToStartOfStatement = Value;
-  }
-  void setCommentString(StringRef Value) { CommentString = Value; }
-};
-
-// Setup a testing class that the GTest framework can call.
-class SystemZAsmLexerTest : public ::testing::Test {
-protected:
-  static void SetUpTestCase() {
-    LLVMInitializeSystemZTargetInfo();
-    LLVMInitializeSystemZTargetMC();
-  }
-
-  std::unique_ptr<MCRegisterInfo> MRI;
-  std::unique_ptr<MockedUpMCAsmInfo> MUPMAI;
-  std::unique_ptr<const MCInstrInfo> MII;
-  std::unique_ptr<MCStreamer> Str;
-  std::unique_ptr<MCAsmParser> Parser;
-
-  std::string TripleName;
-  llvm::Triple Triple;
-  const Target *TheTarget;
-
-  const MCTargetOptions MCOptions;
-  MCObjectFileInfo MOFI;
-
-  // Get the SystemZ Target info.
-  static const Target *getTarget(std::string Triple) {
-    std::string Error;
-    llvm::Triple TripleName(Triple);
-    const Target *TheTarget = TargetRegistry::lookupTarget(Triple, Error);
-    if (!TheTarget)
-      return nullptr;
-
-    return TheTarget;
-  }
-
-  SystemZAsmLexerTest() {
-    // We will use the SystemZ triple, because of missing
-    // Object File and Streamer support for the z/OS target.
-    TripleName = "s390x-ibm-linux";
-    Triple = llvm::Triple(TripleName);
-
-    TheTarget = getTarget(TripleName);
-    EXPECT_NE(TheTarget, nullptr);
-
-    MRI.reset(TheTarget->createMCRegInfo(TripleName));
-    EXPECT_NE(MRI, nullptr);
-
-    std::unique_ptr<MCAsmInfo> MAI;
-    MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
-    EXPECT_NE(MAI, nullptr);
-
-    // Now we cast to our mocked up version of MCAsmInfo.
-    MUPMAI.reset(static_cast<MockedUpMCAsmInfo *>(MAI.release()));
-    // MUPMAI should "hold" MAI.
-    EXPECT_NE(MUPMAI, nullptr);
-    // After releasing, MAI should now be null.
-    EXPECT_EQ(MAI, nullptr);
-  }
-
-  void setupCallToAsmParser(StringRef AsmStr) {
-    std::unique_ptr<MemoryBuffer> Buffer(MemoryBuffer::getMemBuffer(AsmStr));
-    SourceMgr SrcMgr;
-    SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
-
-    llvm::Triple Triple(TripleName);
-    MCContext Ctx(MUPMAI.get(), MRI.get(), &MOFI, &SrcMgr, &MCOptions);
-    MOFI.InitMCObjectFileInfo(Triple, false, Ctx, false);
-
-    Str.reset(TheTarget->createNullStreamer(Ctx));
-
-    Parser.reset(createMCAsmParser(SrcMgr, Ctx, *Str, *MUPMAI));
-    // Lex initially to get the string.
-    Parser->getLexer().Lex();
-  }
-
-  void lexAndCheckTokens(StringRef AsmStr,
-                         SmallVector<AsmToken::TokenKind> ExpectedTokens) {
-    // Get reference to AsmLexer.
-    MCAsmLexer &Lexer = Parser->getLexer();
-    // Loop through all expected tokens checking one by one.
-    for (size_t I = 0; I < ExpectedTokens.size(); ++I) {
-      EXPECT_EQ(Lexer.getTok().getKind(), ExpectedTokens[I]);
-      Lexer.Lex();
-    }
-  }
-};
-
-TEST_F(SystemZAsmLexerTest, CheckDontRestrictCommentStringToStartOfStatement) {
-  StringRef AsmStr = "jne #-4";
-
-  // Setup.
-  setupCallToAsmParser(AsmStr);
-
-  SmallVector<AsmToken::TokenKind> ExpectedTokens(
-      {AsmToken::Identifier, AsmToken::EndOfStatement});
-  lexAndCheckTokens(AsmStr /* "jne #-4" */, ExpectedTokens);
-}
-
-// Testing MCAsmInfo's RestrictCommentStringToStartOfStatement attribute.
-TEST_F(SystemZAsmLexerTest, CheckRestrictCommentStringToStartOfStatement) {
-  StringRef AsmStr = "jne #-4";
-
-  // Setup.
-  MUPMAI->setRestrictCommentStringToStartOfStatement(true);
-  setupCallToAsmParser(AsmStr);
-
-  // When we are restricting the comment string to only the start of the
-  // statement, The sequence of tokens we are expecting are: Identifier - "jne"
-  // Hash - '#'
-  // Minus - '-'
-  // Integer - '4'
-  SmallVector<AsmToken::TokenKind> ExpectedTokens(
-      {AsmToken::Identifier, AsmToken::Hash, AsmToken::Minus,
-       AsmToken::Integer});
-  lexAndCheckTokens(AsmStr /* "jne #-4" */, ExpectedTokens);
-}
-
-// Test HLASM Comment Syntax ('*')
-TEST_F(SystemZAsmLexerTest, CheckHLASMComment) {
-  StringRef AsmStr = "* lhi 1,10";
-
-  // Setup.
-  MUPMAI->setCommentString("*");
-  setupCallToAsmParser(AsmStr);
-
-  SmallVector<AsmToken::TokenKind> ExpectedTokens(
-      {AsmToken::EndOfStatement, AsmToken::Eof});
-  lexAndCheckTokens(AsmStr /* "* lhi 1,10" */, ExpectedTokens);
-}
-} // end anonymous namespace