[llvm] r275058 - Provide support for preserving assembly comments

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 14 14:46:43 PDT 2016 

> On Jul 11, 2016, at 5:42 AM, Nirav Dave via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: niravd
> Date: Mon Jul 11 07:42:14 2016
> New Revision: 275058
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=275058&view=rev
> Log:
> Provide support for preserving assembly comments
> 
> Preserve assembly comments from input in output assembly and flags to
> toggle property. This is on by default for inline assembly and off in
> llvm-mc.
> 
> Parsed comments are emitted immediately before an EOL which generally
> places them on the expected line.
> 
> Reviewers: rtrieu, dwmw2, rnk, majnemer
> 
> Subscribers: llvm-commits
> 
> Differential Revision: http://reviews.llvm.org/D20020
> 
> Added:
>    llvm/trunk/test/MC/AsmParser/inline-comments.ll
>    llvm/trunk/test/MC/AsmParser/preserve-comments.s
> Modified:
>    llvm/trunk/include/llvm/MC/MCAsmInfo.h
>    llvm/trunk/include/llvm/MC/MCStreamer.h
>    llvm/trunk/include/llvm/MC/MCTargetOptions.h
>    llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp
>    llvm/trunk/lib/MC/MCAsmInfo.cpp
>    llvm/trunk/lib/MC/MCAsmStreamer.cpp
>    llvm/trunk/lib/MC/MCParser/AsmParser.cpp
>    llvm/trunk/lib/MC/MCParser/ELFAsmParser.cpp
>    llvm/trunk/lib/MC/MCStreamer.cpp
>    llvm/trunk/tools/llc/llc.cpp
>    llvm/trunk/tools/llvm-mc/llvm-mc.cpp
> 
> Modified: llvm/trunk/include/llvm/MC/MCAsmInfo.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAsmInfo.h?rev=275058&r1=275057&r2=275058&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCAsmInfo.h (original)
> +++ llvm/trunk/include/llvm/MC/MCAsmInfo.h Mon Jul 11 07:42:14 2016
> @@ -362,6 +362,9 @@ protected:
>   /// construction (see LLVMTargetMachine::initAsmInfo()).
>   bool UseIntegratedAssembler;
> 
> +  /// Preserve Comments in assembly
> +  bool PreserveAsmComments;
> +
>   /// Compress DWARF debug sections. Defaults to no compression.
>   DebugCompressionType CompressDebugSections;
> 
> @@ -575,6 +578,14 @@ public:
>     UseIntegratedAssembler = Value;
>   }
> 
> +  /// Return true if assembly (inline or otherwise) should be parsed.
> +  bool preserveAsmComments() const { return PreserveAsmComments; }
> +
> +  /// Set whether assembly (inline or otherwise) should be parsed.
> +  virtual void setPreserveAsmComments(bool Value) {
> +    PreserveAsmComments = Value;
> +  }
> +


Why is this virtual?


>   DebugCompressionType compressDebugSections() const {
>     return CompressDebugSections;
>   }
> 
> Modified: llvm/trunk/include/llvm/MC/MCStreamer.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCStreamer.h?rev=275058&r1=275057&r2=275058&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCStreamer.h (original)
> +++ llvm/trunk/include/llvm/MC/MCStreamer.h Mon Jul 11 07:42:14 2016
> @@ -252,7 +252,7 @@ public:
>   /// correctly?
>   virtual bool isIntegratedAssemblerRequired() const { return false; }
> 
> -  /// \brief Add a textual command.
> +  /// \brief Add a textual comment.
>   ///
>   /// Typically for comments that can be emitted to the generated .s
>   /// file if applicable as a QoI issue to make the output of the compiler
> @@ -274,6 +274,12 @@ public:
>   /// only prints comments, the object streamer ignores it instead of asserting.
>   virtual void emitRawComment(const Twine &T, bool TabPrefix = true);
> 
> +  /// \brief Add explicit comment T. T is required to be a valid
> +  /// comment in the output and does not need to be escaped.
> +  virtual void addExplicitComment(const Twine &T);
> +  /// \brief Emit added explicit comments.
> +  virtual void emitExplicitComments();
> +
>   /// AddBlankLine - Emit a blank line to a .s file to pretty it up.
>   virtual void AddBlankLine() {}
> 
> 
> Modified: llvm/trunk/include/llvm/MC/MCTargetOptions.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCTargetOptions.h?rev=275058&r1=275057&r2=275058&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCTargetOptions.h (original)
> +++ llvm/trunk/include/llvm/MC/MCTargetOptions.h Mon Jul 11 07:42:14 2016
> @@ -36,6 +36,10 @@ public:
>   bool ShowMCEncoding : 1;
>   bool ShowMCInst : 1;
>   bool AsmVerbose : 1;
> +
> +  /// Preserve Comments in Assembly.
> +  bool PreserveAsmComments : 1;
> +

You forgot to add this to the MCTargetOptions ctor.


— 
Mehdi



>   int DwarfVersion;
>   /// getABIName - If this returns a non-empty string this represents the
>   /// textual name of the ABI that we want the backend to use, e.g. o32, or
> 
> Modified: llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp?rev=275058&r1=275057&r2=275058&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp (original)
> +++ llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp Mon Jul 11 07:42:14 2016
> @@ -70,6 +70,8 @@ void LLVMTargetMachine::initAsmInfo() {
>   if (Options.DisableIntegratedAS)
>     TmpAsmInfo->setUseIntegratedAssembler(false);
> 
> +  TmpAsmInfo->setPreserveAsmComments(Options.MCOptions.PreserveAsmComments);
> +
>   if (Options.CompressDebugSections)
>     TmpAsmInfo->setCompressDebugSections(DebugCompressionType::DCT_ZlibGnu);
> 
> 
> Modified: llvm/trunk/lib/MC/MCAsmInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmInfo.cpp?rev=275058&r1=275057&r2=275058&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCAsmInfo.cpp (original)
> +++ llvm/trunk/lib/MC/MCAsmInfo.cpp Mon Jul 11 07:42:14 2016
> @@ -107,6 +107,7 @@ MCAsmInfo::MCAsmInfo() {
>   //   architecture basis.
>   //   - The target subclasses for AArch64, ARM, and X86 handle these cases
>   UseIntegratedAssembler = false;
> +  PreserveAsmComments = true;
> 
>   CompressDebugSections = DebugCompressionType::DCT_None;
> }
> 
> Modified: llvm/trunk/lib/MC/MCAsmStreamer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmStreamer.cpp?rev=275058&r1=275057&r2=275058&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCAsmStreamer.cpp (original)
> +++ llvm/trunk/lib/MC/MCAsmStreamer.cpp Mon Jul 11 07:42:14 2016
> @@ -46,6 +46,7 @@ class MCAsmStreamer final : public MCStr
>   std::unique_ptr<MCCodeEmitter> Emitter;
>   std::unique_ptr<MCAsmBackend> AsmBackend;
> 
> +  SmallString<128> ExplicitCommentToEmit;
>   SmallString<128> CommentToEmit;
>   raw_svector_ostream CommentStream;
> 
> @@ -73,6 +74,8 @@ public:
>   }
> 
>   inline void EmitEOL() {
> +    // Dump Explicit Comments here.
> +    emitExplicitComments();
>     // If we don't have any comments, just emit a \n.
>     if (!IsVerboseAsm) {
>       OS << '\n';
> @@ -112,6 +115,9 @@ public:
> 
>   void emitRawComment(const Twine &T, bool TabPrefix = true) override;
> 
> +  void addExplicitComment(const Twine &T) override;
> +  void emitExplicitComments() override;
> +
>   /// AddBlankLine - Emit a blank line to a .s file to pretty it up.
>   void AddBlankLine() override {
>     EmitEOL();
> @@ -325,6 +331,49 @@ void MCAsmStreamer::emitRawComment(const
>   EmitEOL();
> }
> 
> +void MCAsmStreamer::addExplicitComment(const Twine &T) {
> +  StringRef c = T.getSingleStringRef();
> +  if (c.equals(StringRef(MAI->getSeparatorString())))
> +    return;
> +  if (c.startswith(StringRef("//"))) {
> +    ExplicitCommentToEmit.append("\t");
> +    ExplicitCommentToEmit.append(MAI->getCommentString());
> +    // drop //
> +    ExplicitCommentToEmit.append(c.slice(2, c.size()).str());
> +  } else if (c.startswith(StringRef("/*"))) {
> +    size_t p = 2, len = c.size() - 2;
> +    // emit each line in comment as separate newline.
> +    do {
> +      size_t newp = std::min(len, c.find_first_of("\r\n", p));
> +      ExplicitCommentToEmit.append("\t");
> +      ExplicitCommentToEmit.append(MAI->getCommentString());
> +      ExplicitCommentToEmit.append(c.slice(p, newp).str());
> +      // If we have another line in this comment add line
> +      if (newp < len)
> +        ExplicitCommentToEmit.append("\n");
> +      p = newp + 1;
> +    } while (p < len);
> +  } else if (c.startswith(StringRef(MAI->getCommentString()))) {
> +    ExplicitCommentToEmit.append("\t");
> +    ExplicitCommentToEmit.append(c.str());
> +  } else if (c.front() == '#') {
> +    // # are comments for ## commentString. Output extra #.
> +    ExplicitCommentToEmit.append("\t#");
> +    ExplicitCommentToEmit.append(c.str());
> +  } else
> +    assert(false && "Unexpected Assembly Comment");
> +  // full line comments immediately output
> +  if (c.back() == '\n')
> +    emitExplicitComments();
> +}
> +
> +void MCAsmStreamer::emitExplicitComments() {
> +  StringRef Comments = ExplicitCommentToEmit;
> +  if (!Comments.empty())
> +    OS << Comments;
> +  ExplicitCommentToEmit.clear();
> +}
> +
> void MCAsmStreamer::ChangeSection(MCSection *Section,
>                                   const MCExpr *Subsection) {
>   assert(Section && "Cannot switch to a null section!");
> @@ -510,8 +559,10 @@ void MCAsmStreamer::EmitSymbolDesc(MCSym
> }
> 
> void MCAsmStreamer::EmitSyntaxDirective() {
> -  if (MAI->getAssemblerDialect() == 1)
> -    OS << "\t.intel_syntax noprefix\n";
> +  if (MAI->getAssemblerDialect() == 1) {
> +    OS << "\t.intel_syntax noprefix";
> +    EmitEOL();
> +  }
>   // FIXME: Currently emit unprefix'ed registers.
>   // The intel_syntax directive has one optional argument 
>   // with may have a value of prefix or noprefix.
> 
> Modified: llvm/trunk/lib/MC/MCParser/AsmParser.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCParser/AsmParser.cpp?rev=275058&r1=275057&r2=275058&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCParser/AsmParser.cpp (original)
> +++ llvm/trunk/lib/MC/MCParser/AsmParser.cpp Mon Jul 11 07:42:14 2016
> @@ -626,9 +626,20 @@ const AsmToken &AsmParser::Lex() {
>   if (Lexer.getTok().is(AsmToken::Error))
>     Error(Lexer.getErrLoc(), Lexer.getErr());
> 
> +  // if it's a end of statement with a comment in it
> +  if (getTok().is(AsmToken::EndOfStatement)) {
> +    // if this is a line comment output it.
> +    if (getTok().getString().front() != '\n' &&
> +        getTok().getString().front() != '\r' && MAI.preserveAsmComments())
> +      Out.addExplicitComment(Twine(getTok().getString()));
> +  }
> +
>   const AsmToken *tok = &Lexer.Lex();
> -  // Drop comments here.
> +
> +  // Parse comments here to be deferred until end of next statement.
>   while (tok->is(AsmToken::Comment)) {
> +    if (MAI.preserveAsmComments())
> +      Out.addExplicitComment(Twine(tok->getString()));
>     tok = &Lexer.Lex();
>   }
> 
> 
> Modified: llvm/trunk/lib/MC/MCParser/ELFAsmParser.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCParser/ELFAsmParser.cpp?rev=275058&r1=275057&r2=275058&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCParser/ELFAsmParser.cpp (original)
> +++ llvm/trunk/lib/MC/MCParser/ELFAsmParser.cpp Mon Jul 11 07:42:14 2016
> @@ -188,6 +188,7 @@ bool ELFAsmParser::ParseSectionSwitch(St
>     if (getParser().parseExpression(Subsection))
>       return true;
>   }
> +  Lex();
> 
>   getStreamer().SwitchSection(getContext().getELFSection(Section, Type, Flags),
>                               Subsection);
> 
> Modified: llvm/trunk/lib/MC/MCStreamer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCStreamer.cpp?rev=275058&r1=275057&r2=275058&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCStreamer.cpp (original)
> +++ llvm/trunk/lib/MC/MCStreamer.cpp Mon Jul 11 07:42:14 2016
> @@ -70,6 +70,9 @@ raw_ostream &MCStreamer::GetCommentOS()
> 
> void MCStreamer::emitRawComment(const Twine &T, bool TabPrefix) {}
> 
> +void MCStreamer::addExplicitComment(const Twine &T) {}
> +void MCStreamer::emitExplicitComments() {}
> +
> void MCStreamer::generateCompactUnwindEncodings(MCAsmBackend *MAB) {
>   for (auto &FI : DwarfFrameInfos)
>     FI.CompactUnwindEncoding =
> 
> Added: llvm/trunk/test/MC/AsmParser/inline-comments.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/AsmParser/inline-comments.ll?rev=275058&view=auto
> ==============================================================================
> --- llvm/trunk/test/MC/AsmParser/inline-comments.ll (added)
> +++ llvm/trunk/test/MC/AsmParser/inline-comments.ll Mon Jul 11 07:42:14 2016
> @@ -0,0 +1,88 @@
> +; RUN: llc %s -o - | sed -n -e '/#APP/,/#NO_APP/p' > %t
> +; RUN: sed -n -e 's/^;CHECK://p' %s > %t2
> +; RUN: diff %t %t2
> +
> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-unknown-linux-gnu"
> +
> +; Function Attrs: nounwind uwtable
> +define void @foo() #0 {
> +entry:
> +  call void asm sideeffect "", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:	#NO_APP
> +  call void asm sideeffect " ", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:
> +;CHECK:	#NO_APP
> +  call void asm sideeffect "\0A", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:
> +;CHECK:
> +;CHECK:	#NO_APP
> +  call void asm sideeffect "/*isolated c comment*/", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:	#isolated c comment
> +;CHECK:	#NO_APP
> +  call void asm sideeffect "/**/", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:	#
> +;CHECK:	#NO_APP
> +  call void asm sideeffect "/*comment with\0Anewline*/", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:	#comment with
> +;CHECK:	#newline
> +;CHECK:	#NO_APP
> +  call void asm sideeffect "//isolated line comment", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:	#isolated line comment
> +;CHECK:	#NO_APP
> +  call void asm sideeffect "#isolated line comment", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:	#isolated line comment
> +;CHECK:	#NO_APP
> +   call void asm sideeffect "nop /* after nop */", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:	nop	# after nop 
> +;CHECK:	#NO_APP
> +  call void asm sideeffect "nop // after nop", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:	nop	# after nop
> +;CHECK:	#NO_APP
> +  call void asm sideeffect "nop # after nop", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:	nop	# after nop
> +;CHECK:	#NO_APP
> +  call void asm sideeffect "nop /* after explicit ended nop */", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:	nop	# after explicit ended nop 
> +;CHECK:	#NO_APP
> +  call void asm sideeffect "nop # after explicit ended nop", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:	nop	# after explicit ended nop
> +;CHECK:	#NO_APP
> +  call void asm sideeffect "nop # after explicit end nop", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:	nop	# after explicit end nop
> +;CHECK:	#NO_APP
> +  call void asm sideeffect "/* before nop */ nop", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:	nop	# before nop 
> +;CHECK:	#NO_APP
> +  call void asm sideeffect "//comment with escaped newline\0A", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:	#comment with escaped newline
> +;CHECK:
> +;CHECK:	#NO_APP
> +  call void asm sideeffect "/*0*/xor/*1*/%eax,/*2*/%ecx/*3*///eol", "~{dirflag},~{fpsr},~{flags}"() #0
> +;CHECK:	#APP
> +;CHECK:	xorl	%eax, %ecx	#0	#1	#2	#3	#eol
> +;CHECK:	#NO_APP
> +  ret void
> +}
> +
> +attributes #0 = { nounwind }
> +
> +!llvm.ident = !{!0}
> +
> +!0 = !{!"clang version 3.9.0 (trunk 268625) (llvm/trunk 268631)"}
> 
> Added: llvm/trunk/test/MC/AsmParser/preserve-comments.s
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/AsmParser/preserve-comments.s?rev=275058&view=auto
> ==============================================================================
> --- llvm/trunk/test/MC/AsmParser/preserve-comments.s (added)
> +++ llvm/trunk/test/MC/AsmParser/preserve-comments.s Mon Jul 11 07:42:14 2016
> @@ -0,0 +1,9 @@
> +	#RUN: llvm-mc -preserve-comments -n -triple i386-linux-gnu < %s > %t
> +	#RUN: diff %s %t
> +	.text
> +
> +	nop
> +	#if DIRECTIVE COMMENT
> +	## WHOLE LINE COMMENT
> +	cmpl	$196, %eax	## EOL COMMENT
> +	#endif
> 
> Modified: llvm/trunk/tools/llc/llc.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llc/llc.cpp?rev=275058&r1=275057&r2=275058&view=diff
> ==============================================================================
> --- llvm/trunk/tools/llc/llc.cpp (original)
> +++ llvm/trunk/tools/llc/llc.cpp Mon Jul 11 07:42:14 2016
> @@ -73,6 +73,10 @@ static cl::opt<bool>
> NoIntegratedAssembler("no-integrated-as", cl::Hidden,
>                       cl::desc("Disable integrated assembler"));
> 
> +static cl::opt<bool>
> +    NoPreserveComments("fno-preserve-as-comments", cl::Hidden,
> +                       cl::desc("Preserve Comments in outputted assembly"));
> +
> // Determine optimization level.
> static cl::opt<char>
> OptLevel("O",
> @@ -332,6 +336,7 @@ static int compileModule(char **argv, LL
>   Options.MCOptions.ShowMCEncoding = ShowMCEncoding;
>   Options.MCOptions.MCUseDwarfDirectory = EnableDwarfDirectory;
>   Options.MCOptions.AsmVerbose = AsmVerbose;
> +  Options.MCOptions.PreserveAsmComments = !NoPreserveComments;
> 
>   std::unique_ptr<TargetMachine> Target(
>       TheTarget->createTargetMachine(TheTriple.getTriple(), CPUStr, FeaturesStr,
> 
> Modified: llvm/trunk/tools/llvm-mc/llvm-mc.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-mc/llvm-mc.cpp?rev=275058&r1=275057&r2=275058&view=diff
> ==============================================================================
> --- llvm/trunk/tools/llvm-mc/llvm-mc.cpp (original)
> +++ llvm/trunk/tools/llvm-mc/llvm-mc.cpp Mon Jul 11 07:42:14 2016
> @@ -87,6 +87,10 @@ PrintImmHex("print-imm-hex", cl::init(fa
> static cl::list<std::string>
> DefineSymbol("defsym", cl::desc("Defines a symbol to be an integer constant"));
> 
> +static cl::opt<bool>
> +    PreserveComments("preserve-comments",
> +                     cl::desc("Preserve Comments in outputted assembly"));
> +
> enum OutputFileType {
>   OFT_Null,
>   OFT_AssemblyFile,
> @@ -430,6 +434,7 @@ int main(int argc, char **argv) {
>     }
>     MAI->setCompressDebugSections(CompressDebugSections);
>   }
> +  MAI->setPreserveAsmComments(PreserveComments);
> 
>   // FIXME: This is not pretty. MCContext has a ptr to MCObjectFileInfo and
>   // MCObjectFileInfo needs a MCContext reference in order to initialize itself.
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

More information about the llvm-commits mailing list