[WIP] New AsmPrinterHandler that is able to interleave source code

[rogfer01]

Summary

This is the LLVM side of the proposal in http://lists.llvm.org/pipermail/cfe-dev/2017-February/052549.html

Basically introduces a new AsmPrinterHandler that is enabled when AsmSource is not None. AsmSource is not just a boolean because we do not want to clutter the assembler with debug directives if we are just using -fverbose-asm (and no other debug information has been requested).

See the clang side of this in https://reviews.llvm.org/D30898

*TODO*: Add tests

[fpetrogalli]

Hi Roger,

thank you for working on this. I have some minor comments, good job!

A test would also make clear what would be the desired output of this new functionality.

Francesco

include/llvm/MC/MCTargetOptions.h

26–31

Please describe this enumeration.

lib/CodeGen/AsmPrinter/SourceInterleave.cpp

55–64

You could use DebugLoc::print (or factor out part of it if you don't need column and line number).

99

Nitpicking: const std:string?

107

Nitpicking: const int?

136

}// end namespace llvm

[efriedma]

lib/CodeGen/AsmPrinter/SourceInterleave.cpp

38

If we're compiling source code using clang, we already have the source code in a buffer in memory. Re-reading it from the disk is useless work at best, and might not even work in some cases (for example, clang supports reading C code from stdin).

[hfinkel]

include/llvm/MC/MCTargetOptions.h

29

As I mentioned in the Clang review; this should be some kind of factored-out feature.

lib/CodeGen/AsmPrinter/SourceInterleave.cpp

129

I'd like to see this at greater than line granularity when column information is available. We should be able to have something like this:

// for (int i = 0; i < n; ++i)
//         ^
xor r1, r1, r1

...

// for (int i = 0; i < n; ++i)
//                         ^
cmp r1, r22

Doing this well means figuring out beforehand whether there are instructions with different column values for a particular line. This is a bit more work, but I can say from my experience making llvm-opt-report, it is definitely worth it.

Also, I'd like to see the line numbers prepended to the line. Since the lines don't always appear in order, this is important. Same with file names, when we print a line in a different file from the previously-printed line, we should say so:

// In foo/bar.c
// int x = a + b;
add r4, r5, r6

[hfinkel]

lib/CodeGen/AsmPrinter/SourceInterleave.cpp

38

I think that it is definitely worth considering about more of a handshake here between the frontend and the backend.

That having been said, we have the same problem with stdin code as we do with optimization reports. We just can't support that in a reasonable way right now.

include/llvm/MC/MCTargetOptions.h

 enum class ExceptionHandling {
   None,     /// No exception support
   DwarfCFI, /// DWARF-like instruction based exceptions
   SjLj,     /// setjmp/longjmp based exceptions
   ARM,      /// ARM EHABI
   WinEH,    /// Windows Exception Handling
 };
 
+enum class AsmSourceOutput {
+    None,
+    Alone,
+    WithDebug
+};
+
 class StringRef;
 
 class MCTargetOptions {
 public:
   enum AsmInstrumentation {
     AsmInstrumentationNone,
     AsmInstrumentationAddress
   };
 
   /// Enables AddressSanitizer instrumentation at machine level.
   bool SanitizeAddress : 1;
 
   bool MCRelaxAll : 1;
   bool MCNoExecStack : 1;
   bool MCFatalWarnings : 1;
   bool MCNoWarn : 1;
   bool MCNoDeprecatedWarn : 1;
   bool MCSaveTempLabels : 1;
   bool MCUseDwarfDirectory : 1;
   bool MCIncrementalLinkerCompatible : 1;
   bool MCPIECopyRelocations : 1;
   bool ShowMCEncoding : 1;
   bool ShowMCInst : 1;
   bool AsmVerbose : 1;
+  int AsmSource : 2;
 
   /// Preserve Comments in Assembly.
   bool PreserveAsmComments : 1;
 
   int DwarfVersion = 0;
 
   std::string ABIName;
 
return (ARE_EQUAL(SanitizeAddress) &&
           ARE_EQUAL(MCNoDeprecatedWarn) &&
           ARE_EQUAL(MCSaveTempLabels) &&
           ARE_EQUAL(MCUseDwarfDirectory) &&
           ARE_EQUAL(MCIncrementalLinkerCompatible) &&
           ARE_EQUAL(MCPIECopyRelocations) &&
           ARE_EQUAL(ShowMCEncoding) &&
           ARE_EQUAL(ShowMCInst) &&
           ARE_EQUAL(AsmVerbose) &&
+          ARE_EQUAL(AsmSource) &&
           ARE_EQUAL(DwarfVersion) &&
           ARE_EQUAL(ABIName) &&
           ARE_EQUAL(IASSearchPaths));
 #undef ARE_EQUAL
 }
 
 inline bool operator!=(const MCTargetOptions &LHS, const MCTargetOptions &RHS) {
   return !(LHS == RHS);
 }
 
 } // end namespace llvm
 
 #endif // LLVM_MC_MCTARGETOPTIONS_H

lib/CodeGen/AsmPrinter/AsmPrinter.cpp

 // This file implements the AsmPrinter class.
 //
 //===----------------------------------------------------------------------===//
 
 #include "AsmPrinterHandler.h"
 #include "CodeViewDebug.h"
 #include "DwarfDebug.h"
 #include "DwarfException.h"
+#include "SourceInterleave.h"
 #include "WinException.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"

 static const char *const DbgTimerName = "emit";
 static const char *const DbgTimerDescription = "Debug Info Emission";
 static const char *const EHTimerName = "write_exception";
 static const char *const EHTimerDescription = "DWARF Exception Writer";
 static const char *const CodeViewLineTablesGroupName = "linetables";
 static const char *const CodeViewLineTablesGroupDescription =
   "CodeView Line Tables";
 
+static const char *const SourceInterleaveGroupName = "source printer";
+static const char *const SourceInterleaveGroupDescription = "Interleave source code comments with instructions";
+static const char *const SourceInterleaveTimerName = "interleave_source";
+static const char *const SourceInterleaveTimerDescription = "Source interleaver";
+
 STATISTIC(EmittedInsts, "Number of machine instrs printed");
 
 char AsmPrinter::ID = 0;
 
 typedef DenseMap<GCStrategy*, std::unique_ptr<GCMetadataPrinter>> gcp_map_type;
 static gcp_map_type &getGCMap(void *&P) {
   if (!P)
     P = new gcp_map_type();
EmitInlineAsm(M.getModuleInlineAsm()+"\n",
                   OutContext.getSubtargetCopy(*STI), TM.Options.MCOptions);
     OutStreamer->AddComment("End of file scope inline assembly");
     OutStreamer->AddBlankLine();
   }
 
   if (MAI->doesSupportDebugInformation()) {
     bool EmitCodeView = MMI->getModule()->getCodeViewFlag();
     if (EmitCodeView && (TM.getTargetTriple().isKnownWindowsMSVCEnvironment() ||
-                         TM.getTargetTriple().isWindowsItaniumEnvironment())) {
-      Handlers.push_back(HandlerInfo(new CodeViewDebug(this),
-                                     DbgTimerName, DbgTimerDescription,
-                                     CodeViewLineTablesGroupName,
-                                     CodeViewLineTablesGroupDescription));
+                         TM.getTargetTriple().isWindowsItaniumEnvironment()) &&
+        TM.Options.MCOptions.AsmSource != (int)llvm::AsmSourceOutput::Alone) {
+      Handlers.push_back(HandlerInfo(
+          new CodeViewDebug(this), DbgTimerName, DbgTimerDescription,
+          CodeViewLineTablesGroupName, CodeViewLineTablesGroupDescription));
     }
-    if (!EmitCodeView || MMI->getModule()->getDwarfVersion()) {
+    if ((!EmitCodeView || MMI->getModule()->getDwarfVersion()) &&
+        TM.Options.MCOptions.AsmSource != (int)llvm::AsmSourceOutput::Alone) {
       DD = new DwarfDebug(this, &M);
       DD->beginModule();
       Handlers.push_back(HandlerInfo(DD, DbgTimerName, DbgTimerDescription,
                                      DWARFGroupName, DWARFGroupDescription));
     }
+    if (TM.Options.MCOptions.AsmSource) {
+      Handlers.push_back(HandlerInfo(
+          new SourceInterleave(this), SourceInterleaveTimerName,
+          SourceInterleaveTimerDescription, SourceInterleaveGroupName,
+          SourceInterleaveGroupDescription));
+    }
   }
 
   switch (MAI->getExceptionHandlingType()) {
   case ExceptionHandling::SjLj:
   case ExceptionHandling::DwarfCFI:
   case ExceptionHandling::ARM:
     isCFIMoveForDebugging = true;
     if (MAI->getExceptionHandlingType() != ExceptionHandling::DwarfCFI)

 /// EmitFunctionBody - This method emits the body and trailer for a
 /// function.
 void AsmPrinter::EmitFunctionBody() {
   EmitFunctionHeader();
 
   // Emit target-specific gunk before the function body.
   EmitFunctionBodyStart();
 
-  bool ShouldPrintDebugScopes = MMI->hasDebugInfo();
-
   // Print out code for the function.
   bool HasAnyRealCode = false;
   int NumInstsInFunction = 0;
   for (auto &MBB : *MF) {
     // Print a label for the basic block.
     EmitBasicBlockStart(MBB);
     for (auto &MI : MBB) {
 
       // Print the assembly for the instruction.
       if (!MI.isPosition() && !MI.isImplicitDef() && !MI.isKill() &&
           !MI.isDebugValue()) {
         HasAnyRealCode = true;
         ++NumInstsInFunction;
       }
 
-      if (ShouldPrintDebugScopes) {
-        for (const HandlerInfo &HI : Handlers) {
-          NamedRegionTimer T(HI.TimerName, HI.TimerDescription,
-                             HI.TimerGroupName, HI.TimerGroupDescription,
-                             TimePassesIsEnabled);
-          HI.Handler->beginInstruction(&MI);
-        }
-      }
+        NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
+                           HI.TimerGroupDescription, TimePassesIsEnabled);
 
       if (isVerbose())
         emitComments(MI, OutStreamer->GetCommentOS());
 
       switch (MI.getOpcode()) {
       case TargetOpcode::CFI_INSTRUCTION:
         emitCFIInstruction(MI);
         break;
for (auto &MI : MBB) {
       case TargetOpcode::KILL:
         if (isVerbose()) emitKill(&MI, *this);
         break;
       default:
         EmitInstruction(&MI);
         break;
       }
 
-      if (ShouldPrintDebugScopes) {
-        for (const HandlerInfo &HI : Handlers) {
-          NamedRegionTimer T(HI.TimerName, HI.TimerDescription,
-                             HI.TimerGroupName, HI.TimerGroupDescription,
-                             TimePassesIsEnabled);
-          HI.Handler->endInstruction();
-        }
-      }
-    }
+        NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
+                           HI.TimerGroupDescription, TimePassesIsEnabled);
 
     EmitBasicBlockEnd(MBB);
   }
 
   EmittedInsts += NumInstsInFunction;
   MachineOptimizationRemarkAnalysis R(DEBUG_TYPE, "InstructionCount",
                                       MF->getFunction()->getSubprogram(),
                                       &MF->front());

lib/CodeGen/AsmPrinter/CMakeLists.txt

add_llvm_library(LLVMAsmPrinter
   DwarfFile.cpp
   DwarfStringPool.cpp
   DwarfUnit.cpp
   EHStreamer.cpp
   ErlangGCPrinter.cpp
   OcamlGCPrinter.cpp
   WinException.cpp
   CodeViewDebug.cpp
+  SourceInterleave.cpp
 
   DEPENDS
   intrinsics_gen
   )

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

if (Processed.insert(InlinedVariable(DV, nullptr)).second)
       if (LexicalScope *Scope = LScopes.findLexicalScope(DV->getScope()))
         createConcreteVariable(*Scope, InlinedVariable(DV, nullptr));
   }
 }
 
 // Process beginning of an instruction.
 void DwarfDebug::beginInstruction(const MachineInstr *MI) {
   DebugHandlerBase::beginInstruction(MI);
-  assert(CurMI);
+  if (!CurMI)
+      return;
 
   // Check if source location changes, but ignore DBG_VALUE and CFI locations.
   if (MI->isDebugValue() || MI->isCFIInstruction())
     return;
   const DebugLoc &DL = MI->getDebugLoc();
   // When we emit a line-0 record, we don't update PrevInstLoc; so look at
   // the last line number actually emitted, to see if it was line 0.
   unsigned LastAsmLine =

lib/CodeGen/AsmPrinter/SourceInterleave.h

+//===-- llvm/lib/CodeGen/AsmPrinter/SourceInterleave.h --*- C++ -*--===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file contains support for interleaving source code with assembler.
+//
+//===----------------------------------------------------------------------===//
+
+#include "AsmPrinterHandler.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/IR/DebugLoc.h"
+#include <memory>
+
+namespace llvm {
+
+class SourceInterleave : public AsmPrinterHandler {
+  struct SourceCodeInfo {
+    enum { UnknownFunctionStart = -2 };
+    int LastLineSeen = 0;
+    std::unique_ptr<MemoryBuffer> MemBuff;
+    std::vector<StringRef> LineIndex;
+  };
+
+  llvm::StringMap<std::unique_ptr<SourceCodeInfo>> BufferMap;
+
+  static void indexBuffer(SourceCodeInfo *SI);
+  SourceCodeInfo *getOrCreateSourceCodeInfo(StringRef FullPath);
+
+  static std::string getFullPathname(StringRef Dir, StringRef Fn);
+  static std::string getFullPathname(const DebugLoc &DL);
+  AsmPrinter *AP;
+
+public:
+  SourceInterleave(AsmPrinter *ap) : AP(ap) {}
+
+  void setSymbolSize(const MCSymbol *Sym, uint64_t Size) override {}
+
+  void endModule() override;
+
+  void beginFunction(const MachineFunction *MF) override;
+  void endFunction(const MachineFunction *MF) override;
+
+  void beginInstruction(const MachineInstr *MI) override;
+  void endInstruction() override;
+};
+}

lib/CodeGen/AsmPrinter/SourceInterleave.cpp

+//===-- llvm/lib/CodeGen/AsmPrinter/SourceInterleave.cpp --*- C++ -*--===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file contains support for interleaving source code with assembler.
+//
+//===----------------------------------------------------------------------===//
+
+#include "SourceInterleave.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/CodeGen/AsmPrinter.h"
+#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/IR/DebugInfo.h"
+#include "llvm/IR/Function.h"
+#include "llvm/MC/MCStreamer.h"
+#include "llvm/Support/LineIterator.h"
+#include "llvm/Support/Path.h"
+
+namespace llvm {
+
+void SourceInterleave::indexBuffer(SourceCodeInfo *SI) {
+  SI->LineIndex.reserve(128);
+  SI->LineIndex.insert(SI->LineIndex.end(),
+                       line_iterator(*SI->MemBuff, /* SkipBlanks */ false),
+                       line_iterator());
+}
+
+SourceInterleave::SourceCodeInfo *
+SourceInterleave::getOrCreateSourceCodeInfo(StringRef FullPathname) {
+  auto it = BufferMap.find(FullPathname);
+  if (it == BufferMap.end()) {
+    std::unique_ptr<SourceCodeInfo> SI(new SourceCodeInfo);
+    auto ErrorMemBuffer = MemoryBuffer::getFile(FullPathname);
+    if (ErrorMemBuffer) {
+      SI->MemBuff = std::move(*ErrorMemBuffer);
+      indexBuffer(SI.get());
+    }
+    it = BufferMap.insert({FullPathname, std::move(SI)}).first;
+  }
+  return it->second.get();
+}
+
+std::string SourceInterleave::getFullPathname(StringRef Dir, StringRef Fn) {
+  SmallString<128> FullPathname;
+  llvm::sys::path::append(FullPathname, Dir, Fn);
+
+  return FullPathname.str();
+}
+
+std::string SourceInterleave::getFullPathname(const DebugLoc &DL) {
+  std::string Dir;
+  std::string Fn;
+  if (DIScope *S = DL->getScope()) {
+    Dir = S->getDirectory();
+    Fn = S->getFilename();
+  }
+
+  return getFullPathname(Dir, Fn);
+}
+
+void SourceInterleave::endModule() {}
+
+void SourceInterleave::beginFunction(const MachineFunction *MF) {
+  const Function *F = MF->getFunction();
+  DISubprogram *DS = F->getSubprogram();
+  if (!DS)
+    return;
+
+  if (!DS->getFile())
+    return;
+
+  std::string FullPathname = getFullPathname(DS->getFile()->getDirectory(),
+                                             DS->getFile()->getFilename());
+  SourceCodeInfo *SI = getOrCreateSourceCodeInfo(FullPathname);
+  assert(SI);
+
+  // If we could not load the file, give up.
+  if (SI->MemBuff == nullptr)
+    return;
+
+  // While unlikely, it might happen that we do not really know where this
+  // function starts.
+  if (DS->getLine() > 0)
+    SI->LastLineSeen = DS->getLine() - 1;
+}
+
+void SourceInterleave::endFunction(const MachineFunction *MF) {}
+
+void SourceInterleave::beginInstruction(const MachineInstr *MI) {
+  const DebugLoc &DL = MI->getDebugLoc();
+  if (!DL)
+    return;
+
+  std::string FullPathname = getFullPathname(DL);
+  SourceCodeInfo *SI = getOrCreateSourceCodeInfo(FullPathname);
+  assert(SI);
+
+  // If we could not load the file, give up.
+  if (SI->MemBuff == nullptr)
+    return;
+
+  int CurrentLine = DL->getLine();
+  if (CurrentLine == 0)
+    return;
+
+  // If we have no idea where this function started, use the current line
+  // as a sensible starting point.
+  if (SI->LastLineSeen == SourceCodeInfo::UnknownFunctionStart)
+    SI->LastLineSeen = CurrentLine - 1;
+  // If we're not moving or going backwards, ignore this.
+  else if (CurrentLine - SI->LastLineSeen < 0)
+    return;
+
+  for (int Line = SI->LastLineSeen + 1; Line <= CurrentLine; Line++) {
+    if (Line <= 0 || (unsigned)Line > SI->LineIndex.size())
+      break;
+
+    SmallString<128> Str;
+    raw_svector_ostream OS(Str);
+
+    OS << ' ' << FullPathname << ':' << Line << ":  ";
+    OS << SI->LineIndex[Line - 1].str();
+
+    AP->OutStreamer->emitRawComment(OS.str(), false);
+  }
+
+  SI->LastLineSeen = CurrentLine;
+}
+
+void SourceInterleave::endInstruction() {}
+}

lib/MC/MCTargetOptions.cpp

 
 using namespace llvm;
 
 MCTargetOptions::MCTargetOptions()
     : SanitizeAddress(false), MCRelaxAll(false), MCNoExecStack(false),
       MCFatalWarnings(false), MCNoWarn(false), MCNoDeprecatedWarn(false),
       MCSaveTempLabels(false), MCUseDwarfDirectory(false),
       MCIncrementalLinkerCompatible(false), MCPIECopyRelocations(false),
-      ShowMCEncoding(false), ShowMCInst(false), AsmVerbose(false),
+      ShowMCEncoding(false), ShowMCInst(false), AsmVerbose(false), AsmSource(0),
       PreserveAsmComments(true) {}
 
 StringRef MCTargetOptions::getABIName() const {
   return ABIName;
 }