[PATCH] D151906: [BOLT][DWARF] Fix new debug re-writer

[Maksim Panchenko via Phabricator via llvm-commits]

maksfb added inline comments.


================
Comment at: bolt/include/bolt/Core/DIEBuilder.h:19
 #include "llvm/CodeGen/DIE.h"
 #include "llvm/DWARFLinker/DWARFLinkerCompileUnit.h"
 #include "llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h"
----------------
Do we need this?


================
Comment at: bolt/include/bolt/Core/DIEBuilder.h:56
 
   struct UnitInfo {
+    std::vector<std::unique_ptr<DIEInfo>> DieInfoVector;
----------------
Add comment with a description.


================
Comment at: bolt/include/bolt/Core/DIEBuilder.h:62
+    uint32_t UnitLength = 0;
     bool Isconstructed = false;
     std::unordered_map<uint64_t, uint32_t> DIEIDMap;
----------------



================
Comment at: bolt/include/bolt/Core/DIEBuilder.h:63
     bool Isconstructed = false;
-    uint32_t NewDieId = 0;
     std::unordered_map<uint64_t, uint32_t> DIEIDMap;
   };
----------------
Add a comment.


================
Comment at: bolt/include/bolt/Core/DIEBuilder.h:169
 
   /// Update the Cross-CU reference offset.
   void computeReference();
----------------



================
Comment at: bolt/include/bolt/Core/DIEBuilder.h:170
   /// Update the Cross-CU reference offset.
   void computeReference();
 
----------------



================
Comment at: bolt/include/bolt/Core/DIEBuilder.h:186-187
 
     errs() << "BOLT-ERROR: The DIE is not allocated before looking up, some"
            << "unexpected corner cases happened.\n";
+    return *CloneUnitCtxMap[UnitId].DieInfoVector.front().get();
----------------
Make it an internal DWARF errror?


================
Comment at: bolt/include/bolt/Core/DebugData.h:608
 
-enum class PatcherKind { SimpleBinaryPatcher, DebugInfoBinaryPatcher };
+enum class PatcherKind { SimpleBinaryPatcher };
 /// Abstract interface for classes that apply modifications to a binary string.
----------------
Remove now or in follow up diffs.


================
Comment at: bolt/lib/Core/CMakeLists.txt:8
   Support
+  DWARFLinker
+  BinaryFormat
----------------
Check if we really need it?


================
Comment at: bolt/lib/Core/DIEBuilder.cpp:42
 
-    // If DWARF 4, type unit is store in .debug_types. So we need to calibrate
-    // the begin of .debug_info to the first Compile Unit offset.
-    if (!DWARF4CUVector.empty()) {
-      UnitInfo &FirstCUInfo = getUnitInfoByDwarfUnit(*DWARF4CUVector.front());
-      CUBase = FirstCUInfo.UnitOffset;
-    }
-
-    uint64_t NewAddr =
-        DstDIEInfo->Die->getOffset() + DstUnitInfo.UnitOffset - CUBase;
+    uint64_t NewAddr = DstDIEInfo->Die->getOffset() + DstUnitInfo.UnitOffset;
     SrcDIEInfo->Die->replaceValue(DIEAlloc, Attr, Form, DIEInteger(NewAddr));
----------------



================
Comment at: bolt/lib/Core/DIEBuilder.cpp:585
 
   if (auto OptionalValue = Val.getAsUnsignedConstant())
     Value = *OptionalValue;
----------------
Use braces.


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:74
 
 class DIEStreamer : public DwarfStreamer {
   DIEBuilder *DIEBldr;
----------------
Could you please add comments with the description of the class?


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:360
+
+static void emitDWOBuilder(DIEBuilder &DwoDIEBlder, DWARFRewriter &Rewriter,
+                           const DWARFUnit &SplitCU, const uint64_t DWOId) {
----------------



================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:362
+                           const DWARFUnit &SplitCU, const uint64_t DWOId) {
+  // populate debug_info and debug_abbrev for current dwo into stringRef.
+  DwoDIEBlder.generateAbbrevs();
----------------



================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:1858
+  // GDB-11, DWARF4: TU units from dwo are included.
+  if (MaxDWARFVersion == 5)
+    SkipTypeUnits = !TUListSize ? true
----------------



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151906/new/

https://reviews.llvm.org/D151906