[PATCH] D139214: [BOLT][DWARF] Don't create extra .debug_str_offsets contributions

[Maksim Panchenko via Phabricator via llvm-commits]

maksfb added inline comments.


================
Comment at: bolt/include/bolt/Core/DebugData.h:447
   std::map<uint32_t, uint32_t> IndexToAddressMap;
+  std::unordered_set<uint64_t> ProcessedBaseOffsets;
   // Section size not including header.
----------------
Use `DenseSet<>`.


================
Comment at: bolt/lib/Core/DebugData.cpp:1095
+      findAttributeInfo(Unit.getUnitDIE(), dwarf::DW_AT_str_offsets_base);
+  assert(AttrVal && "DW_AT_str_offsets_base not present.");
+  Optional<uint64_t> Val = AttrVal->V.getAsSectionOffset();
----------------
Is this an assert on internal condition or is it the input validation?


================
Comment at: bolt/lib/Core/DebugData.cpp:1110-1111
+  }
+  // Will assert if we already processed this contribution, and now skipping it,
+  // but it was modified.
+  assert(RetVal.second ||
----------------
When can this happen?


================
Comment at: bolt/test/X86/dwarf5-shared-str-offset-base.s:12
+
+# This test checks that with DWARF5 when two CUs share the same .debug_str_offsets
+# entry we do not create a new one.
----------------
Perhaps include the source files since they appear to be short and how .s was generated.


================
Comment at: bolt/test/X86/dwarf5-shared-str-offset-base.s:13
+# This test checks that with DWARF5 when two CUs share the same .debug_str_offsets
+# entry we do not create a new one.
+
----------------



================
Comment at: bolt/test/X86/dwarf5-shared-str-offset-base.s:291
+.Ldebug_addr_end0:
+	.ident	"Facebook clang version 15.0.0"
+	.section	".note.GNU-stack","", at progbits
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139214