On Sun, Jan 8, 2012 at 12:38 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
> Hi,
> this patch fixes first half of the PR. The PR is about a function that is
> supposed
> to be inlined at -O2. The function contains two calls, one indirect and
> inlining
> the indirect call makes things a lot better. GCC however things that
> replacing call
> by two calls at -O2 is a bad idea.
>
> This patch solves first part of it. The function in question is COMDAT. We
> already
> know that C++ coding style makes COMDATs quite rarely to be shared and at -Os
> we
> already handle them sort of similarly to static ufnctions. We however don't
> do that
> at -O2. This patch makes it happen in both cases.
>
> Bootstrapped/regtested x86_64-linux, comitted.
> Will commit the testcase with the followup fix.
Err ...
> PR tree-optimization/51680
> * ipa-inline.c (want_inline_small_function_p): Be more lax on functions
> whose inlining reduce unit size.
>
> Index: ipa-inline.c
> ===================================================================
> --- ipa-inline.c (revision 182949)
> +++ ipa-inline.c (working copy)
> @@ -482,21 +482,13 @@ want_inline_small_function_p (struct cgr
> e->inline_failed = CIF_MAX_INLINE_INSNS_SINGLE_LIMIT;
> want_inline = false;
> }
> - else if (!DECL_DECLARED_INLINE_P (callee->decl)
> - && !flag_inline_functions)
> - {
> - e->inline_failed = CIF_NOT_DECLARED_INLINED;
> - want_inline = false;
> - }
> - else if (!DECL_DECLARED_INLINE_P (callee->decl)
> - && growth >= MAX_INLINE_INSNS_AUTO)
> - {
> - e->inline_failed = CIF_MAX_INLINE_INSNS_AUTO_LIMIT;
> - want_inline = false;
> - }
> - /* If call is cold, do not inline when function body would grow.
> - Still inline when the overall unit size will shrink because the
> offline
> - copy of function being eliminated.
> + /* Before giving up based on fact that caller size will grow, allow
> + functions that are called few times and eliminating the offline
> + copy will lead to overall code size reduction.
> + Not all of these will be handled by subsequent inlining of functions
> + called once: in particular weak functions are not handled or
> funcitons
> + that inline to multiple calls but a lot of bodies is optimized out.
> + Finally we want to inline earlier to allow inlining of callbacks.
>
> This is slightly wrong on aggressive side: it is entirely possible
> that function is called many times with a context where inlining
> @@ -509,24 +501,39 @@ want_inline_small_function_p (struct cgr
> first, this situation is not a problem at all: after inlining all
> "good" calls, we will realize that keeping the function around is
> better. */
> - else if (!cgraph_maybe_hot_edge_p (e)
> - && (DECL_EXTERNAL (callee->decl)
> + else if (growth <= MAX_INLINE_INSNS_SINGLE
That surely should be MAX_INLINE_INSNS_AUTO instead. At _least_.
You are triggering very much more inlining during early inlining now -
I don't see
where we did this for -Os already as you claim. In fact it is totally
against the spirit of early inlining now :(
Why does IPA inlining not figure out that inlining the indirect call is good?
> + /* Unlike for functions called once, we play unsafe with
> + COMDATs. We can allow that since we know functions
> + in consideration are small (and thus risk is small) and
> + moreover grow estimates already accounts that COMDAT
> + functions may or may not disappear when eliminated from
> + current unit. With good probability making aggressive
> + choice in all units is going to make overall program
> + smaller.
> +
> + Consequently we ask cgraph_can_remove_if_no_direct_calls_p
> + instead of
> + cgraph_will_be_removed_from_program_if_no_direct_calls */
> + && !DECL_EXTERNAL (callee->decl)
> + && cgraph_can_remove_if_no_direct_calls_p (callee)
> + && estimate_growth (callee) <= 0)
> + ;
> + else if (!DECL_DECLARED_INLINE_P (callee->decl)
> + && !flag_inline_functions)
> + {
> + e->inline_failed = CIF_NOT_DECLARED_INLINED;
> + want_inline = false;
> + }
> + else if (!DECL_DECLARED_INLINE_P (callee->decl)
> + && growth >= MAX_INLINE_INSNS_AUTO)
> + {
> + e->inline_failed = CIF_MAX_INLINE_INSNS_AUTO_LIMIT;
> + want_inline = false;
> + }
> + /* If call is cold, do not inline when function body would grow. */
> + else if (!cgraph_maybe_hot_edge_p (e))
>
> - /* Unlike for functions called once, we play unsafe with
> - COMDATs. We can allow that since we know functions
> - in consideration are small (and thus risk is small) and
> - moreover grow estimates already accounts that COMDAT
> - functions may or may not disappear when eliminated from
> - current unit. With good probability making aggressive
> - choice in all units is going to make overall program
> - smaller.
> -
> - Consequently we ask
> cgraph_can_remove_if_no_direct_calls_p
> - instead of
> - cgraph_will_be_removed_from_program_if_no_direct_calls
> */
>
> - || !cgraph_can_remove_if_no_direct_calls_p (callee)
> - || estimate_growth (callee) > 0))
> {
> e->inline_failed = CIF_UNLIKELY_CALL;
> want_inline = false;
