[llvm] 301d926 - [AsmParser][SystemZ][z/OS] Re-introduce HLASM comment syntax
Anirudh Prasad via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 24 07:17:08 PDT 2021
Author: Anirudh Prasad
Date: 2021-03-24T10:17:00-04:00
New Revision: 301d9261b787b25a3b1a80af425fea4366f9330d
URL: https://github.com/llvm/llvm-project/commit/301d9261b787b25a3b1a80af425fea4366f9330d
DIFF: https://github.com/llvm/llvm-project/commit/301d9261b787b25a3b1a80af425fea4366f9330d.diff
LOG: [AsmParser][SystemZ][z/OS] Re-introduce HLASM comment syntax
- https://reviews.llvm.org/rGb605cfb336989705f391d255b7628062d3dfe9c3 was reverted due to sanitizer bugs in the introduced unit-test (specifically in the Address sanitizer https://lab.llvm.org/buildbot/#/builders/5/builds/5697)
- This patch attempts to rectify that, as well as re-factor parts of the test
- The issue was previously, within the `setupCallToAsmParser` function in the unit-test, `SrcMgr` was declared as a local variable. `SrcMgr` owns a unique pointer. Since the variable goes out of scope at the end of the function, the unique pointer is released.
- This patch, moves the declaration of the `SrcMgr` variable to a class field, since the scope will remain until the class's destructor is invoked (which in this case is at the end of the unit test)
- Furthermore, this patch also moves the `MCContext Ctx` declaration from a local variable instance inside a function, to a unique pointer class field. This ensures the instantiation of the MCContext remains until the tear down of the test.
Reviewed By: abhina.sreeskantharajan
Differential Revision: https://reviews.llvm.org/D99004
Added:
llvm/unittests/MC/SystemZ/CMakeLists.txt
llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
Modified:
llvm/include/llvm/MC/MCAsmInfo.h
llvm/lib/MC/MCParser/AsmLexer.cpp
llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCAsmInfo.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/MC/MCAsmInfo.h b/llvm/include/llvm/MC/MCAsmInfo.h
index 309932b29bb1..d97ebb6e4763 100644
--- a/llvm/include/llvm/MC/MCAsmInfo.h
+++ b/llvm/include/llvm/MC/MCAsmInfo.h
@@ -122,10 +122,14 @@ class MCAsmInfo {
/// other when on the same line. Defaults to ';'
const char *SeparatorString;
- /// This indicates the comment character used by the assembler. Defaults to
+ /// This indicates the comment string 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;
@@ -557,6 +561,9 @@ class MCAsmInfo {
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; }
diff --git a/llvm/lib/MC/MCParser/AsmLexer.cpp b/llvm/lib/MC/MCParser/AsmLexer.cpp
index 1fa22ab000f0..dd481d46f788 100644
--- a/llvm/lib/MC/MCParser/AsmLexer.cpp
+++ b/llvm/lib/MC/MCParser/AsmLexer.cpp
@@ -659,6 +659,9 @@ size_t AsmLexer::peekTokens(MutableArrayRef<AsmToken> Buf,
}
bool AsmLexer::isAtStartOfComment(const char *Ptr) {
+ if (MAI.getRestrictCommentStringToStartOfStatement() && !IsAtStartOfStatement)
+ return false;
+
StringRef CommentString = MAI.getCommentString();
if (CommentString.size() == 1)
diff --git a/llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCAsmInfo.cpp b/llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCAsmInfo.cpp
index c537020cdee8..8c4567cd1c4e 100644
--- a/llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCAsmInfo.cpp
+++ b/llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCAsmInfo.cpp
@@ -21,7 +21,8 @@ SystemZMCAsmInfo::SystemZMCAsmInfo(const Triple &TT) {
MaxInstLength = 6;
- CommentString = "#";
+ CommentString = AssemblerDialect == AD_HLASM ? "*" : "#";
+ RestrictCommentStringToStartOfStatement = (AssemblerDialect == AD_HLASM);
ZeroDirective = "\t.space\t";
Data64bitsDirective = "\t.quad\t";
UsesELFSectionDirectiveForBSS = true;
diff --git a/llvm/unittests/MC/SystemZ/CMakeLists.txt b/llvm/unittests/MC/SystemZ/CMakeLists.txt
new file mode 100644
index 000000000000..c50e7db265ce
--- /dev/null
+++ b/llvm/unittests/MC/SystemZ/CMakeLists.txt
@@ -0,0 +1,14 @@
+include_directories(
+ ${LLVM_MAIN_SRC_DIR}/lib/Target/SystemZ
+ )
+
+set(LLVM_LINK_COMPONENTS
+ SystemZ
+ MCParser
+ MC
+ Support
+ )
+
+add_llvm_unittest(SystemZAsmLexerTests
+ SystemZAsmLexerTest.cpp
+ )
diff --git a/llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp b/llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
new file mode 100644
index 000000000000..183b3ffc9352
--- /dev/null
+++ b/llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
@@ -0,0 +1,155 @@
+//===- 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::unique_ptr<MCContext> Ctx;
+
+ SourceMgr SrcMgr;
+ std::string TripleName;
+ llvm::Triple Triple;
+ const Target *TheTarget;
+
+ const MCTargetOptions MCOptions;
+ MCObjectFileInfo MOFI;
+
+ 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);
+
+ std::string Error;
+ TheTarget = TargetRegistry::lookupTarget(TripleName, Error);
+ 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));
+ SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
+ EXPECT_EQ(Buffer, nullptr);
+
+ Ctx.reset(
+ new MCContext(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
More information about the llvm-commits
mailing list