[PATCH] D59902: [COFF] Optimize range extension thunk insertion memory usage

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo accepted this revision.
mstorsjo added a comment.
This revision is now accepted and ready to land.

This looks good from both a performance and memory usage perspective - I'll try to test run it on a largish executable that needs thunks. The code takes a little while to wrap the head around, but I think I understand it.

I tested it on a largish testcase that uses thunks, where linking takes around 24 seconds, where code layout was pretty consistently 2450 ms before this patch and 2500 ms after it. So to my eye, it's a very, very marginal slowdown for the actual thunk cases, but clearly outweighed by everything else.



================
Comment at: lld/COFF/Writer.cpp:460
+      // file. Only add each thunk once to each input file.
+      auto Insertion = ThunkSymtabIndices.insert({{File, Thunk}, ~0U});
+      uint32_t &ThunkSymbolIndex = Insertion.first->second;
----------------
Does the initial value `~0U` actually matter here? If `Insertion.second` is true (if this was the first insertion), the value will be unconditionally overwritten anyway, right? (I guess it's a good sentinel to safeguard against not updating it though, so it's probably ok as such, just trying to grasp all aspects of this.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59902