[AsmParser][SystemZ][z/OS] Introducing HLASM Parser support to AsmParser - Part 1

[anirudhp]

Summary

• This patch (is one in a series of patches) which introduces HLASM Parser support (for the first parameter of inline asm statements) to LLVM (main RFC here)
• This patch in particular introduces HLASM Parser support for Z machine instructions.
• The approach taken here was to subclass AsmParser, and make various functions and variables as "protected" wherever appropriate.
• The HLASMAsmParser class overrides the parseStatement function. Two new private functions parseAsHLASMLabel and parseAsMachineInstruction are introduced as well.

The general syntax is laid out as follows (more information available in HLASM V1R6 Language Reference Manual - Chapter 2 - Instruction Statement Format):

<TokA><spaces.*><TokB><spaces.*><TokC><spaces.*><TokD>

1. TokA is referred to as the Name Entry. This token is optional
2. TokB is referred to as the Operation Entry. This token is mandatory.
3. TokC is referred to as the Operand Entry. This token is mandatory
4. TokD is referred to as the Remarks Entry. This token is optional

• If TokA is provided, then we either parse TokA as a possible comment or as a label (Name Entry), Tok B as the Operation Entry and so on.
• If TokA is not provided (i.e. we have one or more spaces and then the first token), then we will parse the first token (i.e TokB) as a possible Z machine instruction, TokC as the operands to the Z machine instruction and TokD as a possible Remark field
• TokC (Operand Entry), no spaces are allowed between OperandEntries. If a space occurs it is classified as an error.
• TokD if provided is taken as is, and emitted as a comment.

The following additional approach was examined, but not taken:

• Adding custom private only functions to base AsmParser class, and only invoking them for z/OS. While this would eliminate the need for another child class, these private functions would be of non-use to every other target. Similarly, adding any pure virtual functions to the base MCAsmParser class and overriding them in AsmParser would also have the same disadvantage.

Testing:

• This patch doesn't have tests added with it, for the sole reason that MCStreamer Support and Object File support hasn't been added for the z/OS target (yet). Hence, it's not possible generate code outright for the z/OS target. They are in the process of being committed / process of being worked on.

• Any comments / feedback on how to combat this "lack of testing" due to other missing required features is appreciated.

[anirudhp]

• A bit of refactoring (reduced a bit of duplication of code). Removed functions which no longer need to be under "protected".

[uweigand]

llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

511

Maybe just pass MAI into the routine, so it can check those parameters itself?

llvm/lib/MC/MCParser/AsmParser.cpp

6193

I guess now this doesn't really have to be a separate routine any more?

[abhina.sreeskantharajan]

llvm/include/llvm/MC/MCAsmInfo.h

566

nit: Move this between getCommentString and getLabelSuffix to maintain the same order the class members are listed as.

[anirudhp]

• Addressing comments

[anirudhp]

llvm/include/llvm/MC/MCAsmInfo.h

566

Sure. But I'll wait until https://reviews.llvm.org/D97703 is merged in, as its around the same area. When I rebase it, I'll make sure to move it to the right place.

llvm/lib/MC/MCParser/AsmParser.cpp

6193

Yes, you're right. But since the rest of the function is essentially just getTargetParser().ParseInstruction and getTargetParser().MatchAndEmitInstruction(), I didn't want to decompose logic in checkAndGenerateLOCDirectiveForInstruction into the same function. I'd rather have it as a separate routine, since it does something different.

[uweigand]

llvm/lib/MC/MCParser/AsmParser.cpp

6193

My point was that it might be preferable to minimize changes to existing code. Just split the old parseStatement in two parts. Leave the second part (the new parseAndMatchAndEmitTargetInstruction) where it is above. The diff should just show the end of one function and the header of the new function being inserted, the rest of the lines are unchanged.

[anirudhp]

• Addressed Comments x2
• Removing an unneeded function based on refactoring

[anirudhp]

Ping :)

[anirudhp]

Ping Ping! :) :)

[abhina.sreeskantharajan]

I have two nits, but overall LGTM, your patch may need to be rebased because CI is failing

llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

279

nit: Do we need to save this to a bool? We can just do the following if there is only one use.
if (MAI->getEmitGNUAsmStartIndentationMarker())

[anirudhp]

Ping :) :)

[anirudhp]

• Rebasing on latest master + setting the AllowHashInIdentifier in HLASMAsmParser since it has landed.

[anirudhp]

Hi All!
Does anyone have feedback on the patch / approach taken? Any opinions would be greatly welcome and appreciated! :)

[anirudhp]

Ping!! :)

[abhina.sreeskantharajan]

Is it possible to add partial unit tests like you did in your previous patches?

[anirudhp]

In D98276#2705806, @abhina.sreeskantharajan wrote:

Is it possible to add partial unit tests like you did in your previous patches?

No (atleast from the top of my head). The very instantiation of the Parser instance is dependent on the target triple itself. And since we don't really have other object writer/streamer support yet, there's no way to instantiate for z/OS (even in the unit test). Also, writing tests end to test the end to end implementation for the parser would be a tremendous amount of work (comparing it to writing simple mc tests).

[Kai]

llvm/include/llvm/MC/MCAsmInfo.h

130

The comment contradicts the source below.

[anirudhp]

• Rebased on latest to bring in new changes
• Corrected erroneous comment

[anirudhp]

Ping! :)

[Kai]

LGTM.

llvm/lib/Target/SystemZ/AsmParser/SystemZAsmParser.cpp

1377

Nit: If you combine both ifs, then it's more clearer that you just check that there is no space.

[anirudhp]

• Addressing small comment regarding coalescing if statements + rebase on latest.

[anirudhp]

• Rebase on latest master + update various HLASM specific setters from MCAsmLexer.

[anirudhp]

• Rebase on latest + changes to account for API change of MCContext and MCObjectFileInfo

[uweigand]

llvm/lib/MC/MCParser/AsmParser.cpp

6193

Can you please move this new parseAndMatchAndEmitTargetInstruction routine up in the source file to immediately after AsmParser::parseStatement ? That should make the diff smaller and easier to review.

llvm/lib/Target/SystemZ/AsmParser/SystemZAsmParser.cpp

1389–1415

You can avoid the duplicate error message by moving the code in here up before the isNot(Asm::EndOfStatement) check into a separate

if (isParsingHLASM() && getTok().is(AsmToken::Space))

block

1399

Should be skip the leading space(s), or are they intentionally part of the "remark"?

[anirudhp]

• Addressed comments.

[anirudhp]

llvm/lib/Target/SystemZ/AsmParser/SystemZAsmParser.cpp

1399

They are intentionally part of the "Remark" field.

[uweigand]

llvm/lib/MC/MCParser/AsmParser.cpp

2291

Maybe those two lines should move into parseAndMatchAndEmitTargetInstruction, they are identical between the two callers.

2299

Can you rename the parameters so that you don't need to change the code below just because the names are different? Ideally, the rest of the function should have an empty diff to the current source.

2304

Here's some more spurious differences that should be removed.

llvm/lib/Target/SystemZ/AsmParser/SystemZAsmParser.cpp

1386

I suggested to move the HLASM remark handling here (*before* the EndOfStatement check), I still think this would be a better place. Is there any reason why this doesn't work?

[anirudhp]

llvm/lib/MC/MCParser/AsmParser.cpp

2304

clang-tidy complains that the variable i is lower case, when it should be upper case. Is it okay if we ignore clang-tidy in this case in favour of a more unchanged diff or vice-versa? I'm not sure which is preferred / more important.

[uweigand]

llvm/lib/MC/MCParser/AsmParser.cpp

2304

Given that this is already in the current sources, I'd prefer to leave i in lower case. Once the diff is gone, clang-tidy shouldn't run on in anymore anyway ...

[anirudhp]

• Addressed Comments (Moved location of parseAndMatchAndEmitTargetInstruction to make the diff easier to view, changed SystemZAsmParser to emit Remark fields differently)

[uweigand]

llvm/lib/MC/MCParser/AsmParser.cpp

6182

Having two variables that are always the complement of one another seems odd, in particular because ShouldParseAsLabel doesn't appear to be used anywhere?

6203

Why not simply "return parseAsMachineInstruction"?

llvm/lib/Target/SystemZ/AsmParser/SystemZAsmParser.cpp

1396

This still looks odd to me. What seems to be going on is that the LexUntilEndOfStatement lexes everything including the initial space. The explicit Lex then lexes the EndOfStatement marker.

I believe that would be wrong: the EndOfStatement is already lexed below (last line of this function). And the comment just immediately following here seems to imply that the "Lex" was intended to lex that initial *space*. But for that to be true, wouldn't the Lex have to be done *before* the LexUntilEndOfStatement?

[anirudhp]

llvm/lib/MC/MCParser/AsmParser.cpp

6182

I guess I could probably remove the assert itself. The if else block determines that either one of the flags would be set but not both, so I don't see the point in the assert. There's no other path that would would set both of them to true. If a flag is set incorrectly, the parsing logic for it should catch it.

ShouldParseAsLabel doesn't appear to be used anywhere?

Currently, no. But since these patches are staggered, the next sequence of patches once this lands, will add in support for parsing in labels. But I agree with your overall comment, that one variable should do the trick. Two are unnecessary. I will remove the ShouldParseAsLabel flag.

llvm/lib/Target/SystemZ/AsmParser/SystemZAsmParser.cpp

1396

This still looks odd to me. What seems to be going on is that the LexUntilEndOfStatement lexes everything including the initial space. The explicit Lex then lexes the EndOfStatement marker.

Hmm, I don't think this is exactly right. The Lexer's SkipSpace attribute is set to false in AsmParser.cpp. Thus, spaces aren't "ignored" as part of regular tokens, and you have an actual AsmToken::Space.

For example, When we have something like this:
" abc def ghij \n"
The sequence of AsmTokens seen are: Space, Identifier, Space, Identifier, Space, Identifier, Space, EndOfStatement"

Relevant implementation in AsmLexer.cpp (the LexToken() function).

case '\t':
  IsAtStartOfStatement = OldIsAtStartOfStatement;
  while (*CurPtr == ' ' || *CurPtr == '\t')
    CurPtr++;
  if (SkipSpace)
    return LexToken(); // Ignore whitespace.
  else
    return AsmToken(AsmToken::Space, StringRef(TokStart, CurPtr - TokStart));

The CurPtr field always jumps over the space, but if SkipSpace is set to false, we don't lex the "next" token, we just return an AsmToken::Space where the onus is on the user to do with it what they want. If its set to true, we just recursively call LexToken() again to return the next possible non-space AsmToken.

If you take a look at the LexUntilEndOfStatement function:

StringRef AsmLexer::LexUntilEndOfStatement() {
  TokStart = CurPtr;

  while (!isAtStartOfComment(CurPtr) &&     // Start of line comment.
         !isAtStatementSeparator(CurPtr) && // End of statement marker.
         *CurPtr != '\n' && *CurPtr != '\r' && CurPtr != CurBuf.end()) {
    ++CurPtr;
  }
  return StringRef(TokStart, CurPtr-TokStart);
}

CurPtr already points to the first non-space character, and like you mentioned it goes on until the "end of statement" is reached. After that, a StringRef is returned.

So, when you come into the relevant block in the code:

    if (isParsingHLASM() && getTok().is(AsmToken::Space)) {
      // We've confirmed that there is a Remark field.
      StringRef Remark(getLexer().LexUntilEndOfStatement());
      Parser.Lex();
....
....
....

We've confirmed that there is a leading space(s), and we want to basically get everything after a space until the end of the statement, to emit as a comment. Once we've done that however, the current AsmToken is still an AsmToken::Space, so when we call Lex, we're still "lexing" the AsmToken::Space, and returning the next token to be lexed which is now a "\n", because we've effectively moved the CurPtr to the end of statement (in the previous call). The point to note here is that the LexUntilEndOfStatement function doesn't return an AsmToken, it simply returns a StringRef from CurPtr until it encounters the first \n (or the carriage return or the end of the string), while advancing CurPtr. So when we exit the block, we still have a AsmToken::EndOfStatement to be checked.

The other way of looking at it is possibly the following (this should be the same behaviour, but I haven't tried it yet, so some pseudo code).

if (isParsingHLASM() && getTok().is(AsmToken::Space)) {
     // Lex the leading spaces first
     
     // In a loop, lex all tokens until you reach the end of statement token
     // aggregating all the "returned" values into a string.
     // i.e. while (getTok().isNot(AsmToken::EndOfStatement) {
     //     RemarkString += getTok().getString()
     //     Parser.Lex()
     // }

     // Emit aggregated string as a comment 
}

Apologies for the very long comment. Please let me know if I've missed something / misunderstood something.

[anirudhp]

• Addressed comments (removed unneeded variable in AsmParser.cpp and assert)

[anirudhp]

Ping :)

[uweigand]

OK, this looks good to me now. Thanks!

llvm/lib/Target/SystemZ/AsmParser/SystemZAsmParser.cpp

1396

Thanks for the details. I think you're correct here. I guess I was confused because LexUntilEndOfStatement directly refers to CurPtr, which is different from the CurTok maintained by the lexer. But existing users of LexUntilEndOfStatement do it the same as you do here, so that should be fine.

[anirudhp]

• Rebase on latest.

llvm/include/llvm/MC/MCAsmInfo.h

protected:
   /// 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 character used by the assembler.  Defaults to
   /// "#"
   StringRef CommentString;
 
+  /// Should we emit the '\t' as the starting indentation marker for GNU inline
+  /// asm statements. Defaults to false.
+  bool EmitGNUAsmStartIndentationMarker = true;
+
   /// 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:
 
   /// This indicates the column (zero-based) at which asm comments should be
   /// printed.
   unsigned getCommentColumn() const { return 40; }
 
   StringRef getCommentString() const { return CommentString; }
   const char *getLabelSuffix() const { return LabelSuffix; }
 
+  bool getEmitGNUAsmStartIndentationMarker() const {
+    return EmitGNUAsmStartIndentationMarker;
+  }
+
   bool useAssignmentForEHBegin() const { return UseAssignmentForEHBegin; }
   bool needsLocalForSize() const { return NeedsLocalForSize; }
   StringRef getPrivateGlobalPrefix() const { return PrivateGlobalPrefix; }
   StringRef getPrivateLabelPrefix() const { return PrivateLabelPrefix; }
 
   bool hasLinkerPrivateGlobalPrefix() const {
     return !LinkerPrivateGlobalPrefix.empty();
   }

llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

while (*LastEmitted) {
     }
     }
   }
   OS << "\n\t.att_syntax\n" << (char)0;  // null terminate string.
 }
 
 static void EmitGCCInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
                                 MachineModuleInfo *MMI, int AsmPrinterVariant,
+                                bool EmitGNUAsmStartIndentationMarker,
                                 AsmPrinter *AP, unsigned LocCookie,
                                 raw_ostream &OS) {
   int CurVariant = -1;            // The number of the {.|.|.} region we are in.
   const char *LastEmitted = AsmStr; // One past the last character emitted.
   unsigned NumOperands = MI->getNumOperands();
 
+  if (EmitGNUAsmStartIndentationMarker)
     OS << '\t';
 
   while (*LastEmitted) {
     switch (*LastEmitted) {
     default: {
       // Not a special case, emit the string section literally.
       const char *LiteralEnd = LastEmitted+1;
       while (*LiteralEnd && *LiteralEnd != '{' && *LiteralEnd != '|' &&
              *LiteralEnd != '}' && *LiteralEnd != '$' && *LiteralEnd != '\n')
for (unsigned i = MI->getNumOperands(); i != 0; --i) {
     }
   }
 
   // Emit the inline asm to a temporary string so we can emit it through
   // EmitInlineAsm.
   SmallString<256> StringData;
   raw_svector_ostream OS(StringData);
 
+  AsmPrinter *AP = const_cast<AsmPrinter*>(this);
+  if (MI->getInlineAsmDialect() == InlineAsm::AD_ATT) {
     // The variant of the current asmprinter.
     int AsmPrinterVariant = MAI->getAssemblerDialect();
-  AsmPrinter *AP = const_cast<AsmPrinter*>(this);
-  if (MI->getInlineAsmDialect() == InlineAsm::AD_ATT)
-    EmitGCCInlineAsmStr(AsmStr, MI, MMI, AsmPrinterVariant, AP, LocCookie, OS);
-  else
+    // Whether to emit the Start Indentation marker.
+    bool EmitGNUAsmStartIndentationMarker =
+        MAI->getEmitGNUAsmStartIndentationMarker();
+    EmitGCCInlineAsmStr(AsmStr, MI, MMI, AsmPrinterVariant,
+                        EmitGNUAsmStartIndentationMarker, AP, LocCookie, OS);
+  } else
     EmitMSInlineAsmStr(AsmStr, MI, MMI, AP, LocCookie, OS);
 
   // Emit warnings if we use reserved registers on the clobber list, as
   // that might lead to undefined behaviour.
   SmallVector<Register, 8> RestrRegs;
   const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
   // Start with the first operand descriptor, and iterate over them.
   for (unsigned I = InlineAsm::MIOp_FirstOperand, NumOps = MI->getNumOperands();

llvm/lib/MC/MCParser/AsmParser.cpp

private:
   void *SavedDiagContext;
   std::unique_ptr<MCAsmParserExtension> PlatformParser;
   SMLoc StartTokLoc;
 
   /// This is the current buffer index we're lexing from as managed by the
   /// SourceMgr object.
   unsigned CurBuffer;
 
-  AsmCond TheCondState;
-  std::vector<AsmCond> TheCondStack;
-
   /// maps directive names to handler methods in parser
   /// extensions. Extensions register themselves in this map by calling
   /// addDirectiveHandler.
   StringMap<ExtensionDirectiveHandler> ExtensionDirectiveMap;
 
   /// Stack of active macro instantiations.
   std::vector<MacroInstantiation*> ActiveMacros;
 
private:
   bool ParsingMSInlineAsm = false;
 
   /// Did we already inform the user about inconsistent MD5 usage?
   bool ReportedInconsistentMD5 = false;
 
   // Is alt macro mode enabled.
   bool AltMacroMode = false;
 
+protected:
+  virtual bool parseStatement(ParseStatementInfo &Info,
+                              MCAsmParserSemaCallback *SI);
+
+  void printMessage(SMLoc Loc, SourceMgr::DiagKind Kind, const Twine &Msg,
+                    SMRange Range = None) const {
+    ArrayRef<SMRange> Ranges(Range);
+    SrcMgr.PrintMessage(Loc, Kind, Msg, Ranges);
+  }
+
+  /// Should we emit DWARF describing this assembler source?  (Returns false if
+  /// the source has .file directives, which means we don't want to generate
+  /// info describing the assembler source itself.)
+  bool enabledGenDwarfForAssembly();
+
+  /// Emit a .loc directive for the instruction (used when we are generating
+  /// dwarf for the current section).
+  void generateLOCDirectiveForInstruction(SMLoc Loc);
+
+  AsmCond TheCondState;
+  std::vector<AsmCond> TheCondStack;
+
 public:
   AsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out,
             const MCAsmInfo &MAI, unsigned CB);
   AsmParser(const AsmParser &) = delete;
   AsmParser &operator=(const AsmParser &) = delete;
   ~AsmParser() override;
 
   bool Run(bool NoInitialTextSection, bool NoFinalize = false) override;
public:
   bool parseIdentifier(StringRef &Res) override;
   void eatToEndOfStatement() override;
 
   bool checkForValidSection() override;
 
   /// }
 
 private:
-  bool parseStatement(ParseStatementInfo &Info,
-                      MCAsmParserSemaCallback *SI);
   bool parseCurlyBlockScope(SmallVectorImpl<AsmRewrite>& AsmStrRewrites);
   bool parseCppHashLineFilenameComment(SMLoc L, bool SaveLocInfo = true);
 
   void checkForBadMacro(SMLoc DirectiveLoc, StringRef Name, StringRef Body,
                         ArrayRef<MCAsmMacroParameter> Parameters);
   bool expandMacro(raw_svector_ostream &OS, StringRef Body,
                    ArrayRef<MCAsmMacroParameter> Parameters,
                    ArrayRef<MCAsmMacroArgument> A, bool EnableAtPseudoVariable,
private:
   /// Handle exit from macro instantiation.
   void handleMacroExit();
 
   /// Extract AsmTokens for a macro argument.
   bool parseMacroArgument(MCAsmMacroArgument &MA, bool Vararg);
 
   /// Parse all macro arguments for a given macro.
   bool parseMacroArguments(const MCAsmMacro *M, MCAsmMacroArguments &A);
-
   void printMacroInstantiations();
-  void printMessage(SMLoc Loc, SourceMgr::DiagKind Kind, const Twine &Msg,
-                    SMRange Range = None) const {
-    ArrayRef<SMRange> Ranges(Range);
-    SrcMgr.PrintMessage(Loc, Kind, Msg, Ranges);
-  }
   static void DiagHandler(const SMDiagnostic &Diag, void *Context);
 
-  /// Should we emit DWARF describing this assembler source?  (Returns false if
-  /// the source has .file directives, which means we don't want to generate
-  /// info describing the assembler source itself.)
-  bool enabledGenDwarfForAssembly();
-
   /// Enter the specified file. This returns true on failure.
   bool enterIncludeFile(const std::string &Filename);
 
   /// Process the specified file for the .incbin directive.
   /// This returns true on failure.
   bool processIncbinFile(const std::string &Filename, int64_t Skip = 0,
                          const MCExpr *Count = nullptr, SMLoc Loc = SMLoc());
 
private:
   // Directives to support address-significance tables.
   bool parseDirectiveAddrsig();
   bool parseDirectiveAddrsigSym();
 
   void initializeDirectiveKindMap();
   void initializeCVDefRangeTypeMap();
 };
 
+class HLASMAsmParser final : public AsmParser {
+private:
+  MCAsmLexer &Lexer;
+  MCStreamer &Out;
+
+  void lexLeadingSpaces() {
+    while (Lexer.is(AsmToken::Space))
+      Lexer.Lex();
+  }
+
+  bool parseAsHLASMLabel(ParseStatementInfo &Info, MCAsmParserSemaCallback *SI);
+  bool parseAsMachineInstruction(ParseStatementInfo &Info,
+                                 MCAsmParserSemaCallback *SI);
+
+public:
+  HLASMAsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out,
+                 const MCAsmInfo &MAI, unsigned CB = 0)
+      : AsmParser(SM, Ctx, Out, MAI, CB), Lexer(getLexer()), Out(Out) {
+    Lexer.setSkipSpace(false);
+  }
+
+  ~HLASMAsmParser() { Lexer.setSkipSpace(true); }
+
+  bool parseStatement(ParseStatementInfo &Info,
+                      MCAsmParserSemaCallback *SI) override;
+};
+
 } // end anonymous namespace
 
 namespace llvm {
 
 extern MCAsmParserExtension *createDarwinAsmParser();
 extern MCAsmParserExtension *createELFAsmParser();
 extern MCAsmParserExtension *createCOFFAsmParser();
 extern MCAsmParserExtension *createWasmAsmParser();
if (ParentIncludeLoc != SMLoc()) {
       jumpToLoc(ParentIncludeLoc);
       return Lex();
     }
   }
 
   return *tok;
 }
 
+void AsmParser::generateLOCDirectiveForInstruction(SMLoc Loc) {
+  unsigned Line;
+  if (ActiveMacros.empty())
+    Line = SrcMgr.FindLineNumber(Loc, CurBuffer);
+  else
+    Line = SrcMgr.FindLineNumber(ActiveMacros.front()->InstantiationLoc,
+                                 ActiveMacros.front()->ExitBuffer);
+
+  // If we previously parsed a cpp hash file line comment then make sure the
+  // current Dwarf File is for the CppHashFilename if not then emit the
+  // Dwarf File table for it and adjust the line number for the .loc.
+  if (!CppHashInfo.Filename.empty()) {
+    unsigned FileNumber = getStreamer().emitDwarfFileDirective(
+        0, StringRef(), CppHashInfo.Filename);
+    getContext().setGenDwarfFileNumber(FileNumber);
+
+    unsigned CppHashLocLineNo =
+        SrcMgr.FindLineNumber(CppHashInfo.Loc, CppHashInfo.Buf);
+    Line = CppHashInfo.LineNumber - 1 + (Line - CppHashLocLineNo);
+  }
+
+  getStreamer().emitDwarfLocDirective(
+      getContext().getGenDwarfFileNumber(), Line, 0,
+      DWARF2_LINE_DEFAULT_IS_STMT ? DWARF2_FLAG_IS_STMT : 0, 0, 0, StringRef());
+}
+
 bool AsmParser::enabledGenDwarfForAssembly() {
   // Check whether the user specified -g.
   if (!getContext().getGenDwarfForAssembly())
     return false;
   // If we haven't encountered any .file directives (which would imply that
   // the assembler source was produced with debug info already) then emit one
   // describing the assembler source file itself.
   if (getContext().getGenDwarfFileNumber() == 0) {
bool AsmParser::parseStatement(ParseStatementInfo &Info,
 
   if (ParsingMSInlineAsm && (IDVal == "even" || IDVal == "EVEN"))
     Info.AsmRewrites->emplace_back(AOK_EVEN, IDLoc, 4);
   if (checkForValidSection())
     return true;
 
   // Canonicalize the opcode to lower case.
   std::string OpcodeStr = IDVal.lower();
   ParseInstructionInfo IInfo(Info.AsmRewrites);
   bool ParseHadError = getTargetParser().ParseInstruction(IInfo, OpcodeStr, ID,
                                                           Info.ParsedOperands);
   Info.ParseError = ParseHadError;
 
   // Dump the parsed representation, if requested.
   if (getShowParsedOperands()) {
     SmallString<256> Str;
     raw_svector_ostream OS(Str);
     OS << "parsed instruction: [";
     for (unsigned i = 0; i != Info.ParsedOperands.size(); ++i) {
       if (i != 0)
         OS << ", ";
       Info.ParsedOperands[i]->print(OS);
     }
     OS << "]";
 
     printMessage(IDLoc, SourceMgr::DK_Note, OS.str());
   }
 
   // Fail even if ParseInstruction erroneously returns false.
   if (hasPendingError() || ParseHadError)
     return true;
 
   // If we are generating dwarf for the current section then generate a .loc
   // directive for the instruction.
   if (!ParseHadError && enabledGenDwarfForAssembly() &&
       getContext().getGenDwarfSectionSyms().count(
           getStreamer().getCurrentSectionOnly())) {
-    unsigned Line;
-    if (ActiveMacros.empty())
-      Line = SrcMgr.FindLineNumber(IDLoc, CurBuffer);
-    else
-      Line = SrcMgr.FindLineNumber(ActiveMacros.front()->InstantiationLoc,
-                                   ActiveMacros.front()->ExitBuffer);
-
-    // If we previously parsed a cpp hash file line comment then make sure the
-    // current Dwarf File is for the CppHashFilename if not then emit the
-    // Dwarf File table for it and adjust the line number for the .loc.
-    if (!CppHashInfo.Filename.empty()) {
-      unsigned FileNumber = getStreamer().emitDwarfFileDirective(
-          0, StringRef(), CppHashInfo.Filename);
-      getContext().setGenDwarfFileNumber(FileNumber);
-
-      unsigned CppHashLocLineNo =
-        SrcMgr.FindLineNumber(CppHashInfo.Loc, CppHashInfo.Buf);
-      Line = CppHashInfo.LineNumber - 1 + (Line - CppHashLocLineNo);
-    }
-
-    getStreamer().emitDwarfLocDirective(
-        getContext().getGenDwarfFileNumber(), Line, 0,
-        DWARF2_LINE_DEFAULT_IS_STMT ? DWARF2_FLAG_IS_STMT : 0, 0, 0,
-        StringRef());
+    generateLOCDirectiveForInstruction(IDLoc);
   }
 
   // If parsing succeeded, match the instruction.
   if (!ParseHadError) {
     uint64_t ErrorInfo;
     if (getTargetParser().MatchAndEmitInstruction(
             IDLoc, Info.Opcode, Info.ParsedOperands, Out, ErrorInfo,
             getTargetParser().isParsingMSInlineAsm()))
bool AsmParser::parseMSInlineAsm(
   // Emit the remainder of the asm string.
   if (AsmStart != AsmEnd)
     OS << StringRef(AsmStart, AsmEnd - AsmStart);
 
   AsmString = OS.str();
   return false;
 }
 
+bool HLASMAsmParser::parseAsMachineInstruction(ParseStatementInfo &Info,
+                                               MCAsmParserSemaCallback *SI) {
+  AsmToken OperationEntryTok = Lexer.getTok();
+  SMLoc OperationEntryLoc = OperationEntryTok.getLoc();
+  StringRef OperationEntryVal;
+
+  // If we see a new line or carriage return, emit the new line
+  // and lex it.
+  if (OperationEntryTok.is(AsmToken::EndOfStatement)) {
+    if (getTok().getString().front() == '\n' ||
+        getTok().getString().front() == '\r') {
+      Out.AddBlankLine();
+      Lex();
+      return false;
+    }
+  }
+
+  // Attempt to parse the first token as an Identifier
+  if (parseIdentifier(OperationEntryVal)) {
+    if (!TheCondState.Ignore) {
+      Lex(); // always eat a token
+      return Error(OperationEntryLoc, "unexpected token at start of statement");
+    }
+    OperationEntryVal = "";
+  }
+
+  // Once we've parsed the operation entry successfully, lex
+  // any spaces to get to the OperandEntries.
+  lexLeadingSpaces();
+
+  // Canonicalize the opcode to lower case.
+  std::string OpcodeStr = OperationEntryVal.lower();
+  ParseInstructionInfo IInfo(Info.AsmRewrites);
+  bool ParseHadError = getTargetParser().ParseInstruction(
+      IInfo, OpcodeStr, OperationEntryTok, Info.ParsedOperands);
+  Info.ParseError = ParseHadError;
+
+  // Dump the parsed representation, if requested.
+  if (getShowParsedOperands()) {
+    SmallString<256> Str;
+    raw_svector_ostream OS(Str);
+    OS << "parsed instruction: [";
+    for (unsigned I = 0; I != Info.ParsedOperands.size(); ++I) {
+      if (I != 0)
+        OS << ", ";
+      Info.ParsedOperands[I]->print(OS);
+    }
+    OS << "]";
+
+    printMessage(OperationEntryLoc, SourceMgr::DK_Note, OS.str());
+  }
+
+  // Fail even if ParseInstruction erroneously returns false.
+  if (hasPendingError() || ParseHadError)
+    return true;
+
+  // If we are generating dwarf for the current section then generate a .loc
+  // directive for the instruction.
+  if (!ParseHadError && enabledGenDwarfForAssembly() &&
+      getContext().getGenDwarfSectionSyms().count(
+          getStreamer().getCurrentSectionOnly())) {
+    generateLOCDirectiveForInstruction(OperationEntryLoc);
+  }
+
+  // If parsing succeeded, match the instruction.
+  if (!ParseHadError) {
+    uint64_t ErrorInfo;
+    if (getTargetParser().MatchAndEmitInstruction(
+            OperationEntryLoc, Info.Opcode, Info.ParsedOperands, Out, ErrorInfo,
+            getTargetParser().isParsingMSInlineAsm()))
+      return true;
+  }
+
+  return false;
+}
+
+bool HLASMAsmParser::parseStatement(ParseStatementInfo &Info,
+                                    MCAsmParserSemaCallback *SI) {
+  assert(!hasPendingError() && "parseStatement started with pending error");
+
+  // Should the first token be interpreted as a machine instruction.
+  bool ShouldParseAsMachineInstruction = false;
+  // Should the first token be interpreted as a label (ordinary symbol).
+  bool ShouldParseAsLabel = false;
+
+  // If a Name Entry exists, it should occur at the very
+  // start of the string. In this case, we should parse the
+  // first non-space token as a Label.
+  // If the Name entry is missing (i.e. there's some other
+  // token), then we attempt to parse the first non-space
+  // token as a Machine Instruction.
+  if (getTok().is(AsmToken::Space))
+    ShouldParseAsMachineInstruction = true;
+  else
+    ShouldParseAsLabel = true;
+
+  assert(!(ShouldParseAsMachineInstruction && ShouldParseAsLabel) &&
+         "Cannot parse first token  as both a label and an instruction");
+
+  // If we have an EndOfStatement (which includes the target's comment
+  // string) we can appropriately lex it early on)
+  if (Lexer.is(AsmToken::EndOfStatement)) {
+    // if this is a line comment we can drop it safely
+    if (getTok().getString().empty() || getTok().getString().front() == '\r' ||
+        getTok().getString().front() == '\n')
+      Out.AddBlankLine();
+    Lex();
+    return false;
+  }
+
+  // We have established how to parse the inline asm statement.
+  // Now we can safely lex any spaces to get to the
+  // first token.
+  lexLeadingSpaces();
+
+  if (ShouldParseAsMachineInstruction) {
+    if (parseAsMachineInstruction(Info, SI))
+      return true;
+    return false;
+  }
+
+  // Label parsing support isn't implemented completely (yet).
+  SMLoc Loc = getTok().getLoc();
+  eatToEndOfStatement();
+  return Error(Loc, "HLASM Label parsing support not yet implemented");
+}
+
 namespace llvm {
 namespace MCParserUtils {
 
 /// Returns whether the given symbol is used anywhere in the given expression,
 /// or subexpressions.
 static bool isSymbolUsedInExpression(const MCSymbol *Sym, const MCExpr *Value) {
   switch (Value->getKind()) {
   case MCExpr::Binary: {

 
 } // end namespace MCParserUtils
 } // end namespace llvm
 
 /// Create an MCAsmParser instance.
 MCAsmParser *llvm::createMCAsmParser(SourceMgr &SM, MCContext &C,
                                      MCStreamer &Out, const MCAsmInfo &MAI,
                                      unsigned CB) {
+  if (C.getObjectFileInfo()->getTargetTriple().isOSzOS() &&
+      MAI.getAssemblerDialect())
+    return new HLASMAsmParser(SM, C, Out, MAI, CB);
+
   return new AsmParser(SM, C, Out, MAI, CB);
 }

llvm/lib/Target/SystemZ/AsmParser/SystemZAsmParser.cpp

if (getLexer().isNot(AsmToken::EndOfStatement)) {
     // Read the first operand.
     if (parseOperand(Operands, Name)) {
       return true;
     }
 
     // Read any subsequent operands.
     while (getLexer().is(AsmToken::Comma)) {
       Parser.Lex();
+
+      if (getMAIAssemblerDialect()) {
+        if (getLexer().is(AsmToken::Space))
+          return Error(
+              Parser.getTok().getLoc(),
+              "No space allowed between comma that separates operand entries");
+      }
+
       if (parseOperand(Operands, Name)) {
         return true;
       }
     }
     if (getLexer().isNot(AsmToken::EndOfStatement)) {
       SMLoc Loc = getLexer().getLoc();
+      if (getMAIAssemblerDialect()) {
+        // Under the HLASM variant, we could have the remark field
+        // The remark field occurs after the operation entries
+        // There is a space that separates the operation entries and the
+        // remark field.
+
+        // We don't have a space, we immediately error out.
+        if (getTok().isNot(AsmToken::Space))
+          return Error(Loc, "unexpected token in argument list");
+
+        // We've confirmed that there is a Remark field.
+        StringRef Remark(getLexer().LexUntilEndOfStatement());
+        Parser.Lex();
+
+        // If there is nothing after the space, then there is nothing to emit
+        // We could have a situation as this:
+        // "  \n"
+        // After lexing, we will have
+        // "\n"
+        // This isn't an explicit remark field, so we don't have to output
+        // this as a comment.
+        if (Remark.size())
+          // Output the entire Remarks Field as a comment
+          getStreamer().AddComment(Remark);
+      }
+      else
         return Error(Loc, "unexpected token in argument list");
     }
   }
 
   // Consume the EndOfStatement.
   Parser.Lex();
   return false;
 }
 

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;
 
+  EmitGNUAsmStartIndentationMarker =
+      (AssemblerDialect == AD_HLASM) ? false : true;
   CommentString = "#";
   ZeroDirective = "\t.space\t";
   Data64bitsDirective = "\t.quad\t";
   UsesELFSectionDirectiveForBSS = true;
   SupportsDebugInformation = true;
   ExceptionsType = ExceptionHandling::DwarfCFI;
 }