[PATCH] D36199: [Inliner] Increase threshold for hot callsites without PGO.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

In https://reviews.llvm.org/D36199#829818, @eraman wrote:

> In https://reviews.llvm.org/D36199#828523, @chandlerc wrote:
>
> > While the total size increase doesn't concern me much, the >10% code size growth in some benchmarks is a bit concerning. Do these benchmarks also improve performance? If so, then this might be fine in general (once the -Os behavior is fixed, see below). However, if the benchmarks that are growing in size by a lot aren't also getting faster, then it seems a hard sell at https://reviews.llvm.org/owners/package/2/ where we expect size increase to be at least generally associated with performance wins.
> >
> > Naturally, this seems completely fine at https://reviews.llvm.org/owners/package/3/ either way.
>
>
> There are 5 tests in the test-suite with text size increase > 10% and > 4K. Three of them improve and two don't.  For performance numbers, I used the perf tool to collect uops_retired:any on an Intel Xeon E5-2690 since the running time of these are small and noisy.
>
> Test -  Size increase KB (%)  -  Performance improvement
>
> MultiSource/Benchmarks/VersaBench/beamformer - 5K(41.6%) -  None
>  SingleSource/Benchmarks/Linpack/linpack-pc - 7K(36.4%) - +2%
>  MultiSource/Benchmarks/FreeBench/pifft - 18K(33%) - None
>  MultiSource/Benchmarks/tramp3d-v4 - 148K(20.7%) - 18%
>  SingleSource/Benchmarks/Adobe-C++/loop_unroll - 44K(17.6%) - 9%
>
> The C/C++ subset of SPEC2006 increases by 0.5%. Two benchmarks improve in performance:  453.povray (+1.5%) and 473.astar (+1.81%).


First and foremost, thanks for the excellent data and carefully gathering all of it. =]

> If you think this is iffy for https://reviews.llvm.org/owners/package/2/, I'll do this only for https://reviews.llvm.org/owners/package/3/ for now and work on tuning it further with the goal of enabling it at https://reviews.llvm.org/owners/package/2/.

Yeah, I would put it in https://reviews.llvm.org/owners/package/3/ for now, essentially to be conservative. I think this is a borderline case, but it seems easier to revisit it later than deal with churn of regressing people now, and it seems like a clear win for https://reviews.llvm.org/owners/package/3/.


https://reviews.llvm.org/D36199