[PATCH] D18042: Create a SymbolBody for locals

[Rui Ueyama via llvm-commits]

ruiu added a comment.

LGTM

I build three more programs with and without the series of patches for local symbols (and in addition to that with GNU gold) and observed that the patch series improved performance in all three cases.

Time is in milliseconds.

Clang (RelWithDebugInfo)

  w/o patch  13730
  w/patch    13224
  gold       15795

Clang (RelWithDebugInfo without -f{function,data}-sections)

  w/o patch  13300
  w/patch    12790
  gold       15185

gold in binutils 2.26

  w/o patch  542
  w/patch    501
  gold       726

(The gap between our linker and gold is getting narrower, but that's not a topic of this patch.)

It cannot describe why there is a case that this patch had a negative impact on performance, but I confirmed that the your test result is correct using the archive file you sent to me.  So there must be some hidden condition that this approach is not very good at it. But broadly speaking this seems a good one. This makes things much simpler and seems to improve performance in general.

I hoped you did not change the signature of getSymbol in this patch because that change made this patch a lot larger. You could have done that in a different patch. However, since I have already reviewed all the changes, this overall looks good to me.


http://reviews.llvm.org/D18042