[PATCH] D139638: [llvm-dwarfutil] Add accelerator tables to dwarfutil
Jonas Devlieghere via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 4 13:00:13 PST 2023
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.
LGTM
================
Comment at: llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp:343-353
+ SmallVector<DwarfLinkerAccelTableKind> AccelTables;
+
+ switch (Options.AccelTableKind) {
+ case DwarfUtilAccelKind::None:
+ // Nothing to do.
+ break;
+ case DwarfUtilAccelKind::DWARF:
----------------
avl wrote:
> JDevlieghere wrote:
> > Do we still need a list here? I presume this is a remnant of having the debug_pub{names,types} which could co-exist. It seems unlikely that without those we'll ever need more than one concurrent accelerator table type.
> >
> > This would also make it easier to improve the warning below (line 377) and specify what type of accelerator table the input will be replaced by.
> Usage of list of accelerator tables was introduced not only for debug_pub{names,type} tables. It would be useful if/when additional tables like .gdb_index and/or .debug_cu_index would be implemented. So it looks it would be good to preserve this interface.
>
> if the warning on 377 line looks unclear, how about replacing it with something like this :
>
> Accelerator tables Table1, Table2 will be replaced with Table3, Table 4?
>
>
Ok. I don't feel strongly about the warning, but I think if it's easy to be explicit about the tables that we're generating, that'd be nice.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139638/new/
https://reviews.llvm.org/D139638
More information about the llvm-commits
mailing list