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

Wed Jan 4 12:58:47 PST 2023

avl added inline comments.

================
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:
----------------
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?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139638