[PATCH] D126087: [BOLT][DWARF] Fix TU Index handling for DWARF4/5.
Maksim Panchenko via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon May 23 16:57:36 PDT 2022
maksfb added a comment.
> Added support for handling it when DWP output or DWP is output for DWARF4. For DWARF5 it just handles it as an input for now.
Could you please rephrase?
================
Comment at: bolt/include/bolt/Rewrite/DWARFRewriter.h:31
DWARFRewriter() = delete;
+ using DebugTypesSignaturesPerCU =
+ std::unordered_map<uint64_t, std::unordered_set<uint64_t>>;
----------------
Or `DebugTypesSignaturesPerCUMap`.
================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:738
+ else
+ llvm_unreachable("DW_AT_signature form is not supported.");
}
----------------
If you are validating the input here, then it's better to issue an error as `llvm_unreachable()` has no effect in no-assert builds.
================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:1259
+};
+using TUContributionVector = std::vector<TUContribution>;
+/// Extracts TU entries per CU from DWP.
----------------
================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:1261
+/// Extracts TU entries per CU from DWP.
+static StringRef extractDWOTUFromDWP(
+ DWARFRewriter::DebugTypesSignaturesPerCU &TypeSignaturesPerCU,
----------------
Could you add detailed comments? E.g., it's unclear which parameters are input and which are output.
================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:1289
+ }
+ return StringRef(reinterpret_cast<const char *>(DWOTUSection.c_str()),
+ DWOTUSection.size());
----------------
Is `reinterpret_cast<>` necessary here? Does `StringRef(DWOTUSection)` not work? Do you actually need to return `StringRef` from the function, as it's always a reference to `DWOTUSection` which is a parameter to the function?
================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:1402
+ } else if (Version != CU->getVersion()) {
+ llvm_unreachable("Incompatible DWARF compile unit versions.");
+ }
----------------
Should this be an error instead?
================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:1505
+ if (!TUContributionsToCU.empty()) {
+ auto Index = getContributionIndex(DW_SECT_EXT_TYPES, IndexVersion);
+ for (const TUContribution &TUC : TUContributionsToCU) {
----------------
================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:1615
+ if (IsDWP && SectionName == "debug_types.dwo") {
+ assert(TUIndex &&
+ "DWP Input with .debug_types.dwo section with TU Index.");
----------------
Likewise, should this be an error?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126087/new/
https://reviews.llvm.org/D126087
More information about the llvm-commits
mailing list