[PATCH] D112059: Fix inline builtin handling in case of redefinition

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D112059#3094464 <https://reviews.llvm.org/D112059#3094464>, @nickdesaulniers wrote:

> Here's a [hastily and poorly written] script to measure the average cycle counts for 30 invocations using linux `perf`: https://gist.github.com/nickdesaulniers/4a20ba10c26ac2ad02cb0425b8b0f826
>
> For Diff 382671 (latest; storing redecl state), builds of the linux kernel x86_64 defconfig+CONFIG_FORTIFY_SOURCE=y:
>
>   $ /tmp/measure_30.sh 'make LLVM=1 -j72' 'make LLVM=1 -j72 clean'
>   ...
>   Average of 30 runs: 10780685107280.83 cycles
>
> For Diff 382344 (earlier; walking redecl chain), builds of the linux kernel x86_64 defconfig+CONFIG_FORTIFY_SOURCE=y:
>
>   $ /tmp/measure_30.sh 'make LLVM=1 -j72' 'make LLVM=1 -j72 clean'
>   ...
>   Average of 30 runs: 10745227016663.00 cycles
>
> Damn, so what I proposed was slower, at least for the major case that I care about. I suspect that perhaps there's more forward declarations than actual functions we end up generating IR for, perhaps...either way, I'm sorry for suggesting the "storing redecl state" approach.  If we want to go back to the earlier version in Diff 382344, at this point, I'd be happy to accept that revision.  Sorry for causing whiplash on this.

Thank you for measuring this as well! And no worries on the whiplash (easy for me to say as a reviewer, hah) -- I think it was a reasonable thought to explore and measure the performance of. FWIW, I'd be happy accepting the earlier revision as well.


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

https://reviews.llvm.org/D112059