[PATCH] D53261: [BPF] Add BTF generation for BPF target

Adrian Prantl via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 15 08:46:39 PDT 2018


aprantl added inline comments.


================
Comment at: include/llvm/MC/MCBTFContext.h:75
+
+#define BTF_INFO_KIND(info) (((info) >> 24) & 0x0f)
+#define BTF_INFO_VLEN(info) ((info)&0xffff)
----------------
Could you convert this into a static function?


================
Comment at: include/llvm/MC/MCBTFContext.h:78
+
+#define BTF_KIND_UNKN 0        // Unknown
+#define BTF_KIND_INT 1         // Integer
----------------
Could you convert this into an enum with doxygen comments?


================
Comment at: include/llvm/MC/MCBTFContext.h:161
+// .BTF string subsections.
+struct btf_ext_header {
+  uint16_t magic;
----------------
Please add doxygen comments to all new data types.


================
Comment at: include/llvm/MC/MCBTFContext.h:242
+// BTF_KIND_INT
+class BTFTypeEntryInt : public BTFTypeEntry {
+  unsigned IntVal; // encoding, offset, bits
----------------
... and classes :-)


================
Comment at: include/llvm/MC/MCBTFContext.h:338
+  }
+  std::string &getStringAtOffset(size_t Offset) {
+    return Table[OffsetToIdMap[Offset]];
----------------
StringRef instead of std::string& ?


================
Comment at: include/llvm/MC/MCObjectFileInfo.h:210
 
+  // BTF specific sections.
+  MCSection *BTFSection;
----------------
The proper way to document a group in doxygen is
```
/// BTF specific sections.
/// @{
MCSection *BTFSection;
MCSection *BTFExtSection;
/// @}
```


================
Comment at: lib/CodeGen/AsmPrinter/Dwarf2BTF.h:24
+
+#define BTF_INVALID_ENCODING 0xff
+
----------------
enum please


================
Comment at: lib/CodeGen/AsmPrinter/Dwarf2BTF.h:26
+
+class Die2BTFEntry {
+protected:
----------------
Please document the purpose of this class.


================
Comment at: lib/CodeGen/AsmPrinter/Dwarf2BTF.h:29
+  const DIE &Die;
+  size_t Id; // type index in the BTF list, started from 1
+  struct btf_type BTFType;
----------------
`///`


================
Comment at: lib/CodeGen/AsmPrinter/Dwarf2BTF.h:35
+  // Return desired BTF_KIND for the Die, return BTF_KIND_UNKN for
+  // invalid/unsupported Die
+  static unsigned char getDieKind(const DIE &Die);
----------------
`///` 


================
Comment at: lib/MC/MCBTFContext.cpp:97
+
+  // emit header
+  emitCommonHeader(MCOS);
----------------
Please use full sentences:
`// Emit header.`



================
Comment at: test/MC/BPF/btf-func-line-1.ll:3
+; RUN: llc -march=bpfeb -filetype=obj -o - %s | llvm-readelf -x ".BTF" -x ".BTF.ext" | FileCheck -check-prefixes=CHECK,CHECK-EB %s
+
+; Source code:
----------------
Please add a comment explaining what is being tested here.


Repository:
  rL LLVM

https://reviews.llvm.org/D53261





More information about the llvm-commits mailing list