[PATCH] D139638: [llvm-dwarfutil] Add accelerator tables to dwarfutil

[Jonas Devlieghere via Phabricator via llvm-commits]

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