[PATCH] D49493: [DebugInfo] Reduce debug_str_offsets section size

[Pavel Labath via Phabricator via llvm-commits]

labath added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2407
+  AccelDebugNames.addName(
+      Holder.getStringPool().getEntry(*Asm, Name, /*Indexed*/ false), Die);
 }
----------------
JDevlieghere wrote:
> labath wrote:
> > JDevlieghere wrote:
> > > Would it make sense to make false the default?
> > I wouldn't recommend it as I think it increases the chance of getting things wrong. (e.g. when writing this patch I definitely wanted to make the arg explicit so I could audit each usage for the correct value of the argument).
> > 
> > If you're worried about verbosity, one way I've considered addressing this would be to have an `AccelTable::addName` overload which accepts a StringPool and call's getEntry itself. This would have the extra advantage that the decision of whether to create an Indexed entry or not is moved closer to the code which does the actual emission (the part that knows whether it needs the index or not).
> That's what I figured, but I agree a strong interface is worth being a little more verbose. The overload sounds like a good idea. 
I tried that out, but I wasn't too happy with the result. I've had to move `DwarfStringPool.h` from `lib/` to the `include/` folder and add an extra `AsmPrinter` (in addition to the `StringPool`) argument to the `addName` method.

Instead I propose to do something like <https://reviews.llvm.org/D49542>, which is to keep the StringPool code in DwarfDebug, but collapse all of these `getEntry` calls into one.


Repository:
  rL LLVM

https://reviews.llvm.org/D49493