[PATCH] D81784: [ELF] Resolve relocations in .debug_* referencing (discarded symbols or ICF folded section symbols) to tombstone values

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: lld/ELF/InputSection.cpp:932
+      // address selection entry). Use -2 instead.
+      if (name == ".debug_loc" || name == ".debug_ranges")
+        --tombstone;
----------------
MaskRay wrote:
> avl wrote:
> > I think it is better not to do string comparisons for every relocation pointing to deleted code. 
> > It is worth to move this name comparisons before the loop.
> I think it is actually more efficient doing string comparisons here. The tombstone value code path is rare. By doing string comparisons here, we don't perform string comparisons in the normal code paths.
But you have to do it repeatedly for every relocation in this section, yes? - and .debug_loc and .debug_ranges may contain many relocations.

The test for "is this a debug section" happens unconditionally once, outside the relocation application loop - it seems like this test could/should happen in the same place (& it can be conditional on "is this a debug section":

`
bool isDebugLocOrRanges = isDebugLoc && name == ".debug_loc" || name == ".debug_ranges";
`

Seems like it'd be cheap enough (known short length, the string's already hot in the cache, etc). (alternatively the test could be computed lazily and cached - but given "isDebug" isn't done that way I don't think this one should be either)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81784