public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Kewen.Lin" <linkw@linux.ibm.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Cc: "Jakub Jelinek" <jakub@redhat.com>,
	"Jan Hubicka" <hubicka@ucw.cz>,
	"Richard Biener" <richard.guenther@gmail.com>,
	"Martin Liška" <mliska@suse.cz>
Subject: PING^1 [PATCH] inline: Rebuild target option node for caller [PR105459]
Date: Thu, 23 Jun 2022 10:03:03 +0800	[thread overview]
Message-ID: <05835d48-50e9-be2c-2b60-1b42e9c6e5d6@linux.ibm.com> (raw)
In-Reply-To: <75d4b8f9-5ed2-de4c-d606-510a66714c9a@linux.ibm.com>

Hi,

Gentle ping https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596212.html

BR,
Kewen

on 2022/6/6 14:20, Kewen.Lin via Gcc-patches wrote:
> Hi,
> 
> PR105459 exposes one issue in inline_call handling that when it
> decides to copy FP flags from callee to caller and rebuild the
> optimization node for caller fndecl, it's possible that the target
> option node is also necessary to be rebuilt.  Without updating
> target option node early, it can make nodes share the same target
> option node wrongly, later when we want to unshare it somewhere
> (like in target hook) it can get unexpected results, like ICE on
> uninitialized secondary member of target globals exposed in this PR.
> 
> Commit r12-3721 makes it get exact fp_expression info and causes
> more optimization chances then exposes this issue.  Commit r11-5855
> introduces two target options to shadow flag_excess_precision and
> flag_unsafe_math_optimizations and shows the need to rebuild target
> node in inline_call when optimization node changes.
> 
> As commented in PR105459, I tried to postpone init_function_start
> in cgraph_node::expand, but abandoned it since I thought it just
> concealed the issue.  And I also tried to adjust the target node
> when current function switching, but failed since we get the NULL
> cfun and fndecl in WPA phase.
> 
> Bootstrapped and regtested on x86_64-redhat-linux, powerpc64-linux-gnu
> P8 and powerpc64le-linux-gnu P9.
> 
> Any thoughts?  Is it OK for trunk?
> 
> BR,
> Kewen
> -----
> 
> 	PR tree-optimization/105459
> 
> gcc/ChangeLog:
> 
> 	* ipa-inline-transform.cc (inline_call): Rebuild target option node
> 	once optimization node gets rebuilt.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/lto/pr105459_0.c: New test.
> ---
>  gcc/ipa-inline-transform.cc           | 50 +++++++++++++++++++++++++--
>  gcc/testsuite/gcc.dg/lto/pr105459_0.c | 35 +++++++++++++++++++
>  2 files changed, 83 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/lto/pr105459_0.c
> 
> diff --git a/gcc/ipa-inline-transform.cc b/gcc/ipa-inline-transform.cc
> index 07288e57c73..edba58377f4 100644
> --- a/gcc/ipa-inline-transform.cc
> +++ b/gcc/ipa-inline-transform.cc
> @@ -52,6 +52,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "ipa-modref.h"
>  #include "symtab-thunks.h"
>  #include "symtab-clones.h"
> +#include "target.h"
> 
>  int ncalls_inlined;
>  int nfunctions_inlined;
> @@ -469,8 +470,53 @@ inline_call (struct cgraph_edge *e, bool update_original,
>      }
> 
>    /* Reload global optimization flags.  */
> -  if (reload_optimization_node && DECL_STRUCT_FUNCTION (to->decl) == cfun)
> -    set_cfun (cfun, true);
> +  if (reload_optimization_node)
> +    {
> +      /* Only need to check and update target option node
> +	 when target_option_default_node is not NULL.  */
> +      if (target_option_default_node)
> +	{
> +	  /* Save the current context for optimization and target option
> +	     node.  */
> +	  tree old_optimize
> +	    = build_optimization_node (&global_options, &global_options_set);
> +	  tree old_target_opt
> +	    = build_target_option_node (&global_options, &global_options_set);
> +
> +	  /* Restore optimization with new optimizatin node.  */
> +	  tree new_optimize = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (to->decl);
> +	  if (old_optimize != new_optimize)
> +	    cl_optimization_restore (&global_options, &global_options_set,
> +				     TREE_OPTIMIZATION (new_optimize));
> +
> +	  /* Restore target option with the one from caller fndecl.  */
> +	  tree cur_target_opt = DECL_FUNCTION_SPECIFIC_TARGET (to->decl);
> +	  if (!cur_target_opt)
> +	    cur_target_opt = target_option_default_node;
> +	  cl_target_option_restore (&global_options, &global_options_set,
> +				    TREE_TARGET_OPTION (cur_target_opt));
> +
> +	  /* Update target option as optimization changes.  */
> +	  targetm.target_option.override ();
> +
> +	  /* Rebuild target option node for caller fndecl and replace
> +	     with it if the node changes.  */
> +	  tree new_target_opt
> +	    = build_target_option_node (&global_options, &global_options_set);
> +	  if (cur_target_opt != new_target_opt)
> +	    DECL_FUNCTION_SPECIFIC_TARGET (to->decl) = new_target_opt;
> +
> +	  /* Restore the context with previous saved nodes.  */
> +	  if (old_optimize != new_optimize)
> +	    cl_optimization_restore (&global_options, &global_options_set,
> +				     TREE_OPTIMIZATION (old_optimize));
> +	  cl_target_option_restore (&global_options, &global_options_set,
> +				    TREE_TARGET_OPTION (old_target_opt));
> +	}
> +
> +      if (DECL_STRUCT_FUNCTION (to->decl) == cfun)
> +	set_cfun (cfun, true);
> +    }
> 
>    /* If aliases are involved, redirect edge to the actual destination and
>       possibly remove the aliases.  */
> diff --git a/gcc/testsuite/gcc.dg/lto/pr105459_0.c b/gcc/testsuite/gcc.dg/lto/pr105459_0.c
> new file mode 100644
> index 00000000000..c799e6ef23d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/lto/pr105459_0.c
> @@ -0,0 +1,35 @@
> +/* { dg-lto-do link } */
> +/* { dg-lto-options { { -flto -O1 } } } */
> +
> +double m;
> +int n;
> +
> +__attribute__ ((optimize ("-funsafe-math-optimizations")))
> +void
> +bar (int x)
> +{
> +  n = x;
> +  m = n;
> +}
> +
> +__attribute__ ((flatten))
> +void
> +foo (int x)
> +{
> +  bar (x);
> +}
> +
> +void
> +quux (void)
> +{
> +  ++n;
> +}
> +
> +int
> +main (void)
> +{
> +  foo (0);
> +  quux ();
> +
> +  return 0;
> +}
> --
> 2.27.0

  reply	other threads:[~2022-06-23  2:03 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-06  6:20 Kewen.Lin
2022-06-23  2:03 ` Kewen.Lin [this message]
2022-07-01  8:40   ` PING^1 " Richard Biener
2022-07-01  9:20     ` Jan Hubicka
2022-07-01 12:09       ` Richard Biener
2022-07-01  9:42     ` Kewen.Lin
2022-07-08 11:37 ` Martin Liška
2022-07-11  3:37   ` Kewen.Lin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=05835d48-50e9-be2c-2b60-1b42e9c6e5d6@linux.ibm.com \
    --to=linkw@linux.ibm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=jakub@redhat.com \
    --cc=mliska@suse.cz \
    --cc=richard.guenther@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).