[PATCH] D53736: [BTF] Add BTF DebugInfo

[Yonghong Song via Phabricator via llvm-commits]

yonghong-song added a comment.

@aprantl I just prototyped  to move BTF generation to BPF backend. This indeed works.  The BPF backend
already extends AsmPrinter (in BPFAsmPrinter), which makes changes easier.
The main changes are below:

1. Add an override interface EmitBTF in AsmPrinter and change HandlerInfo to protected as EmitBTF will access this member.

diff --git a/include/llvm/CodeGen/AsmPrinter.h b/include/llvm/CodeGen/AsmPrinter.h
index 14bc0816782..19ff4c2270d 100644

- a/include/llvm/CodeGen/AsmPrinter.h

+++ b/include/llvm/CodeGen/AsmPrinter.h
@@ -137,6 +137,7 @@ private:

  static char ID;
   

+protected:

  struct HandlerInfo {
    AsmPrinterHandler *Handler;
    const char *TimerName;

@@ -425,6 +426,11 @@ public:

  /// instructions in verbose mode.
  virtual void emitImplicitDef(const MachineInstr *MI) const;
   

+  /// Targets can override this to output BTF.
+  /// Currently, only BPF target does this.
+  virtual void EmitBTF(const char *DbgTimerName, const char *DbgTimerDescription,
+    const char *GroupName, const char *GroupDescription) {};
+

2. Move BTFDebug.{cpp,h} to lib/Target/BPF directory. And AsmPrinter.cpp change

looks like below:
diff --git a/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 1a5bbd06033..8b1332ba9a5 100644

- a/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

+++ b/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -12,8 +12,6 @@
 //===----------------------------------------------------------------------===//

#include "llvm/CodeGen/AsmPrinter.h"
-#include "AsmPrinterHandler.h"
-#include "BTFDebug.h"
 #include "CodeViewDebug.h"
 #include "DwarfDebug.h"
 #include "DwarfException.h"
@@ -37,6 +35,7 @@
 #include "llvm/BinaryFormat/COFF.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/BinaryFormat/ELF.h"
+#include "llvm/CodeGen/AsmPrinterHandler.h"
 #include "llvm/CodeGen/GCMetadata.h"
 #include "llvm/CodeGen/GCMetadataPrinter.h"
 #include "llvm/CodeGen/GCStrategy.h"
@@ -313,12 +312,8 @@ bool AsmPrinter::doInitialization(Module &M) {

    Handlers.push_back(HandlerInfo(DD, DbgTimerName, DbgTimerDescription,
                                   DWARFGroupName, DWARFGroupDescription));
  }

- const Triple &TT = TM.getTargetTriple();
- if (TT.getArch() == Triple::bpfel || TT.getArch() == Triple::bpfeb) {
- Handlers.push_back(HandlerInfo(new BTFDebug(this),
- DbgTimerName, DbgTimerDescription,
- BTFGroupName, BTFGroupDescription));
- }

+
+    EmitBTF(DbgTimerName, DbgTimerDescription, BTFGroupName, BTFGroupDescription);

  }
   
  switch (MAI->getExceptionHandlingType()) {

3. Since BPFDebug extends DebugHandlerBase, so I have to move lib/CodeGen/AsmPrinter/DebugHandlerBase.h

to include/llvm/CodeGen directory. The same happens lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.h
and lib/CodeGen/AsmPrinter/AsmPrinterHandler.h to include/llvm/CodeGen directory.
As a result, a lot of files under lib/CodeGen/AsmPrinter needs to be changed to reflect the new include
location.

4. The BPFAsmPrinter simply implements the override function.

diff --git a/lib/Target/BPF/BPFAsmPrinter.cpp b/lib/Target/BPF/BPFAsmPrinter.cpp
index 705211b486b..6dc25f7d708 100644

- a/lib/Target/BPF/BPFAsmPrinter.cpp

+++ b/lib/Target/BPF/BPFAsmPrinter.cpp
@@ -16,6 +16,7 @@
 #include "BPFInstrInfo.h"
 #include "BPFMCInstLower.h"
 #include "BPFTargetMachine.h"
+#include "BTFDebug.h"
 #include "InstPrinter/BPFInstPrinter.h"
 #include "llvm/CodeGen/AsmPrinter.h"
 #include "llvm/CodeGen/MachineConstantPool.h"
@@ -49,6 +50,8 @@ public:

                             raw_ostream &O) override;
   
  void EmitInstruction(const MachineInstr *MI) override;

+  void EmitBTF(const char *DbgTimerName, const char *DbgTimerDescription,
+    const char *GroupName, const char *GroupDescription) override;
 };
 } // namespace

@@ -131,6 +134,13 @@ void BPFAsmPrinter::EmitInstruction(const MachineInstr *MI) {

  EmitToStreamer(*OutStreamer, TmpInst);

}

+void BPFAsmPrinter::EmitBTF(const char *DbgTimerName, const char *DbgTimerDescription,
+  const char *GroupName, const char *GroupDescription) {
+  Handlers.push_back(HandlerInfo(new BTFDebug(this),
+                                 DbgTimerName, DbgTimerDescription,
+                                 GroupName, GroupDescription));
+}
+
 // Force static initialization.

I think this approach is better. But since it will move a few private header files to public include, 
maybe you can comment whether this is okay or not. If it is considered at this point moving these private headers to public headers is premature,
I can implement cmake detection mechanism and we can refactor later if necessary.
Please let me know.


Repository:
  rL LLVM

https://reviews.llvm.org/D53736