| Message ID | 3972590.ZWEW9T1TNL@arcturus.home |
|---|---|
| State | New |
| Headers | show |
| Series | More precise message with -Winline | expand |
On Wed, Jul 24, 2019 at 11:29 AM Eric Botcazou <ebotcazou@adacore.com> wrote: > > Hi, > > for EH cleanups, the branch probability heuristics can consider that edges are > never taken, i.e. their profile count is set to zero. In this case, no matter > how you tweak the inlining parameters, you cannot get function calls inlined, > but -Winline nevertheless prints the standard message about unlikely calls as > in the cases when tweaking the inlining parameters can work. > > Therefore the attached patch adds a new CIF code with the warning message: > > "call is never executed and code size would grow [-Winline]" > > for this case, thus signaling the user that he'd better not waste time trying > to get the call inlined. > > Tested on x86_64-suse-linux, OK for the mainline? Looks good besides + if (e->count.ipa () == profile_count::zero ()) + e->inline_failed = CIF_NEVER_CALL; does it actually matter what kind of profile quality e->count.ipa has compared to profile_count::zero ()? Also +/* Call is considered never executed. */ +DEFCIFCODE(NEVER_CALL, CIF_FINAL_NORMAL, + N_("call is never executed and code size would grow")) suggests the call is never executed, but we only assume that (or the profile training run never hit it). Richard. > > > 2019-07-24 Eric Botcazou <ebotcazou@adacore.com> > > * cif-code.def (NEVER_CALL): New code. > * ipa-inline.c (want_inline_small_function_p): Fix formatting issues. > Set the failure to CIF_NEVER_CALL if the IPA count is zero. > > -- > Eric Botcazou
> Looks good besides > > + if (e->count.ipa () == profile_count::zero ()) > + e->inline_failed = CIF_NEVER_CALL; > > does it actually matter what kind of profile quality e->count.ipa has > compared to profile_count::zero ()? I don't really know, the other examples in the function use e->count.ipa too. > Also > > +/* Call is considered never executed. */ > +DEFCIFCODE(NEVER_CALL, CIF_FINAL_NORMAL, > + N_("call is never executed and code size would grow")) > > suggests the call is never executed, but we only assume that > (or the profile training run never hit it). OK, I added "considered" as in the comment above.
Eric Botcazou <ebotcazou@adacore.com> writes: > Hi, > > for EH cleanups, the branch probability heuristics can consider that edges are > never taken, i.e. their profile count is set to zero. In this case, no matter > how you tweak the inlining parameters, you cannot get function calls inlined, > but -Winline nevertheless prints the standard message about unlikely calls as > in the cases when tweaking the inlining parameters can work. > > Therefore the attached patch adds a new CIF code with the warning message: > > "call is never executed and code size would grow [-Winline]" That seems misleading, unless the exception is really never thrown. Maybe s/never/rarely/ -Andi
> That seems misleading, unless the exception is really never thrown.
See my earlier answer to Richard.
Index: cif-code.def =================================================================== --- cif-code.def (revision 273480) +++ cif-code.def (working copy) @@ -83,6 +83,10 @@ DEFCIFCODE(RECURSIVE_INLINING, CIF_FINAL_NORMAL, DEFCIFCODE(UNLIKELY_CALL, CIF_FINAL_NORMAL, N_("call is unlikely and code size would grow")) +/* Call is considered never executed. */ +DEFCIFCODE(NEVER_CALL, CIF_FINAL_NORMAL, + N_("call is never executed and code size would grow")) + /* Function is not declared as inline. */ DEFCIFCODE(NOT_DECLARED_INLINED, CIF_FINAL_NORMAL, N_("function not declared inline and code size would grow")) Index: ipa-inline.c =================================================================== --- ipa-inline.c (revision 273480) +++ ipa-inline.c (working copy) @@ -810,7 +810,7 @@ want_inline_small_function_p (struct cgraph_edge * | INLINE_HINT_loop_stride)) && !(big_speedup = big_speedup_p (e))))) { - e->inline_failed = CIF_MAX_INLINE_INSNS_SINGLE_LIMIT; + e->inline_failed = CIF_MAX_INLINE_INSNS_SINGLE_LIMIT; want_inline = false; } else if (!DECL_DECLARED_INLINE_P (callee->decl) @@ -818,12 +818,12 @@ want_inline_small_function_p (struct cgraph_edge * && growth >= PARAM_VALUE (PARAM_MAX_INLINE_INSNS_SMALL)) { /* growth_likely_positive is expensive, always test it last. */ - if (growth >= MAX_INLINE_INSNS_SINGLE + if (growth >= MAX_INLINE_INSNS_SINGLE || growth_likely_positive (callee, growth)) { - e->inline_failed = CIF_NOT_DECLARED_INLINED; + e->inline_failed = CIF_NOT_DECLARED_INLINED; want_inline = false; - } + } } /* Apply MAX_INLINE_INSNS_AUTO limit for functions not declared inline Upgrade it to MAX_INLINE_INSNS_SINGLE when hints suggests that @@ -839,12 +839,12 @@ want_inline_small_function_p (struct cgraph_edge * && !(big_speedup == -1 ? big_speedup_p (e) : big_speedup)) { /* growth_likely_positive is expensive, always test it last. */ - if (growth >= MAX_INLINE_INSNS_SINGLE + if (growth >= MAX_INLINE_INSNS_SINGLE || growth_likely_positive (callee, growth)) { 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 (!e->maybe_hot_p () @@ -851,7 +851,10 @@ want_inline_small_function_p (struct cgraph_edge * && (growth >= MAX_INLINE_INSNS_SINGLE || growth_likely_positive (callee, growth))) { - e->inline_failed = CIF_UNLIKELY_CALL; + if (e->count.ipa () == profile_count::zero ()) + e->inline_failed = CIF_NEVER_CALL; + else + e->inline_failed = CIF_UNLIKELY_CALL; want_inline = false; } }