[PATCH] D121876: [BOLT][DWARF] Implementation of monolithic DWARF5.

[Alexander Yermolovich via Phabricator via llvm-commits]

ayermolo added inline comments.


================
Comment at: bolt/lib/Core/BinaryContext.cpp:1465
+    ContainsDwarfLegacy |= CU->getVersion() < 5;
   }
 
----------------
Add a warning if it's dwar5 and legacy


================
Comment at: bolt/lib/Core/BinaryContext.cpp:1503
     const uint64_t CUID = CU->getOffset();
-    getDwarfLineTable(CUID).setLabel(Ctx->getOrCreateSymbol(
+    DWARFDataExtractor LineData(CU->getLineSection().Data, true, 0);
+    DwarfLineTable &BoltLineTable = getDwarfLineTable(CUID);
----------------
remove


================
Comment at: bolt/lib/Core/BinaryContext.cpp:1504
+    DWARFDataExtractor LineData(CU->getLineSection().Data, true, 0);
+    DwarfLineTable &BoltLineTable = getDwarfLineTable(CUID);
+    BoltLineTable.setLabel(Ctx->getOrCreateSymbol(
----------------
rename BinaryLineTable


================
Comment at: bolt/lib/Core/DebugData.cpp:412
+  for (std::unique_ptr<DWARFUnit> &CU : BC->DwCtx->compile_units()) {
+    uint64_t CUId = CU->getOffset();
+    uint8_t AddrSize = CU->getAddressByteSize();
----------------
add const to this and AddrSize


================
Comment at: bolt/lib/Core/DebugData.cpp:419
+      Iter = AddressMaps.insert({CUId, AddressForDWOCU()}).first;
+      auto BaseOffset = CU->getAddrOffsetSectionBase();
+      if (!BaseOffset)
----------------
add type


================
Comment at: bolt/lib/Core/DebugData.cpp:431
+      uint32_t Index = 0;
+      for (uint64_t Addr : AddrTable.getAddressEntries()) {
+        Iter->second.insert(Addr, Index++);
----------------
remove curly brases


================
Comment at: bolt/lib/Core/DebugData.cpp:454
+    uint32_t Counter = 0;
+    auto WriteAddress = [&](uint64_t Address) -> void {
+      ++Counter;
----------------
lower case


================
Comment at: bolt/lib/Core/DebugData.cpp:1523
+    if (Err) {
+      errs() << "BOLT-ERROR: could not extract string from .debug_line_str.";
+      continue;
----------------
remove .


================
Comment at: bolt/lib/Core/DebugData.cpp:1556
 
+  uint16_t DwarfVersion = BC.Ctx->getDwarfVersion();
   // Handle the rest of the Compile Units.
----------------
const


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:375
 
     DWARFDie DIE(&Unit, &Die);
     switch (DIE.getTag()) {
----------------
remove space


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:577
+              LocListIndex = DebugLoclistWriter::InvalidIndex;
+            } else if (Form == dwarf::DW_FORM_loclistx)
+              LocListIndex = Value.getRawUValue();
----------------
use braces for rest


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:588
                  "unexpected DW_AT_location form");
-          if (Unit.isDWOUnit()) {
+          if (Unit.isDWOUnit() || Unit.getVersion()) {
             ArrayRef<uint8_t> Expr = *Value.getAsBlock();
----------------
Unit.getVersion()== 5


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:603
+
               uint64_t Index = Expr.getRawOperand(0);
               Optional<object::SectionedAddress> EntryAddress =
----------------
const


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:610
+              if (Expr.getCode() == dwarf::DW_OP_addrx) {
+                uint32_t EncodingSize =
+                    Expr.getOperandEndOffset(0) - PrevOffset - SizeOfOpcode;
----------------
const here and Index


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:660
+            // index.
+            // TODO: We can now re-write .debug_info can simplify this by just
+            // getting new index and creating a patch.
----------------
rewrite to make sense


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:665
+          } else if (Form == dwarf::DW_FORM_addrx) {
+            uint32_t Index = AddrWriter->getIndexFromAddress(
+                NewAddress ? NewAddress : Address, getCUId(Unit));
----------------
const


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:960
+
+      if (AttrValGnu || AttrVal)
+        DebugInfoPatcher.addLE32Patch(AttrOffset, static_cast<int32_t>(Offset),
----------------
use curly


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:971
+        DebugInfoPatcher.insertNewEntry(DIE, static_cast<int32_t>(Offset));
+      } else
+        assert(
----------------
use curly


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:972
+      } else
+        assert(
+            false &&
----------------
llvm_unreachable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121876