Re: [PATCH] strub: use opt_for_fn during ipa

[Richard Biener <rguenther@suse.de>]

On Thu, 14 Dec 2023, Alexandre Oliva wrote:

> 
> Instead of global optimization levels and flags, check per-function
> ones.
> 
> Regstrapped on x86_64-linux-gnu, also testing on sparc-solaris2.11.3.
> Ok to install?

You have to be generally careful when working within IPA
with function bodies without push/pop_cfun around that, several APIs
have variants with struct function sepcified, using the wrong one
will get you a NULL cfun which _some_ of them handle gracefully and
"wrong", all EH stuff is amongst this for example.

I see you replace flag_exceptions with opt_for_fn (cfun->decl, 
flag_exceptions), given that's 'cfun' this replacement is a no-op
given 'cfun' would be NULL in IPA context unless you pushed a function.

Looking at the 2nd hunk and the caller it seems the transform is
a no-op for indrect_calls but not callees, thus that hunk is OK.

I'll note that push/pop_cfun can be quite expensive so it's worth
avoiding it when possible (we do allow quite some IL manipulation
from "outside" of the 'cfun', you just have to be careful with APIs)

Thanks,
Richard.

> (sorry, Richi, I dropped the ball and failed to fix this before the
> monster commit)
> 
> 
> for  gcc/ChangeLog
> 
> 	* ipa-strub.cc (gsi_insert_finally_seq_after_call): Likewise.
> 	(pass_ipa_strub::adjust_at_calls_call): Likewise.
> ---
>  gcc/ipa-strub.cc |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc
> index 943bb60996fc1..32e2869cf7840 100644
> --- a/gcc/ipa-strub.cc
> +++ b/gcc/ipa-strub.cc
> @@ -2132,7 +2132,7 @@ gsi_insert_finally_seq_after_call (gimple_stmt_iterator gsi, gimple_seq seq)
>  		    || (call && gimple_call_nothrow_p (call))
>  		    || (eh_lp <= 0
>  			&& (TREE_NOTHROW (cfun->decl)
> -			    || !flag_exceptions)));
> +			    || !opt_for_fn (cfun->decl, flag_exceptions))));
>  
>    if (noreturn_p && nothrow_p)
>      return;
> @@ -2470,9 +2470,11 @@ pass_ipa_strub::adjust_at_calls_call (cgraph_edge *e, int named_args,
>    /* If we're already within a strub context, pass on the incoming watermark
>       pointer, and omit the enter and leave calls around the modified call, as an
>       optimization, or as a means to satisfy a tail-call requirement.  */
> -  tree swmp = ((optimize_size || optimize > 2
> +  tree swmp = ((opt_for_fn (e->caller->decl, optimize_size)
> +		|| opt_for_fn (e->caller->decl, optimize) > 2
>  		|| gimple_call_must_tail_p (ocall)
> -		|| (optimize == 2 && gimple_call_tail_p (ocall)))
> +		|| (opt_for_fn (e->caller->decl, optimize) == 2
> +		    && gimple_call_tail_p (ocall)))
>  	       ? strub_watermark_parm (e->caller->decl)
>  	       : NULL_TREE);
>    bool omit_own_watermark = swmp;
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)