public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Fix gamess miscompare
@ 2021-11-17 21:06 Jan Hubicka
  2021-11-18 14:16 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Jan Hubicka @ 2021-11-17 21:06 UTC (permalink / raw)
  To: gcc-patches, mjambor

Hi,
this patch fixes bug in streaming in modref access tree that now cause a failure
of gamess benchmark.  The bug is quite old (present in GCC11 release) but it
needs quite interesting series of events to manifest. In particular
 1) At lto time ISRA turns some parameters passed by reference to scalar
 2) At lto time modref computes summaries for old parameters and then updates
    them but does so quite stupidly believing that the load from parameters
    are now unkonwn loads (rather than optimized out).
    This renders summary not very useful since it thinks every memory aliasing
    int is now accssed (as opposed as parameter dereference)
 3) At stream in we notice too early that summary is useless, set every_access
    flag and drop the list.  However while reading rest of the summary we
    overwrite the flag back to 0 which makes us to lose part of summary.
 4) right selection of partitions needs to be done to avoid late modref from
    recalculating and thus fixing the summary.

This patch fixes the stream in bug, however we also should fix updating of
summaries.  Martin, would be possible to extend get_original_index by "deref"
parameter that would be set to true when refernce was turned to scalar?

Bootstrapped/regtested x86_64-linux. Comitted.

gcc/ChangeLog:

2021-11-17  Jan Hubicka  <hubicka@ucw.cz>

	PR ipa/103246
	* ipa-modref.c (read_modref_records): Fix streaminig in of every_access
	flag.

diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 9ceecdd479f..c94f0589d44 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -3460,10 +3460,10 @@ read_modref_records (lto_input_block *ib, struct data_in *data_in,
 	  size_t every_access = streamer_read_uhwi (ib);
 	  size_t naccesses = streamer_read_uhwi (ib);
 
-	  if (nolto_ref_node)
-	    nolto_ref_node->every_access = every_access;
-	  if (lto_ref_node)
-	    lto_ref_node->every_access = every_access;
+	  if (nolto_ref_node && every_access)
+	    nolto_ref_node->collapse ();
+	  if (lto_ref_node && every_access)
+	    lto_ref_node->collapse ();
 
 	  for (size_t k = 0; k < naccesses; k++)
 	    {

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Fix gamess miscompare
  2021-11-17 21:06 Fix gamess miscompare Jan Hubicka
@ 2021-11-18 14:16 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2021-11-18 14:16 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: GCC Patches, Martin Jambor

On Wed, Nov 17, 2021 at 10:09 PM Jan Hubicka via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
> this patch fixes bug in streaming in modref access tree that now cause a failure
> of gamess benchmark.  The bug is quite old (present in GCC11 release) but it
> needs quite interesting series of events to manifest. In particular
>  1) At lto time ISRA turns some parameters passed by reference to scalar
>  2) At lto time modref computes summaries for old parameters and then updates
>     them but does so quite stupidly believing that the load from parameters
>     are now unkonwn loads (rather than optimized out).
>     This renders summary not very useful since it thinks every memory aliasing
>     int is now accssed (as opposed as parameter dereference)
>  3) At stream in we notice too early that summary is useless, set every_access
>     flag and drop the list.  However while reading rest of the summary we
>     overwrite the flag back to 0 which makes us to lose part of summary.
>  4) right selection of partitions needs to be done to avoid late modref from
>     recalculating and thus fixing the summary.
>
> This patch fixes the stream in bug, however we also should fix updating of
> summaries.  Martin, would be possible to extend get_original_index by "deref"
> parameter that would be set to true when refernce was turned to scalar?
>
> Bootstrapped/regtested x86_64-linux. Comitted.

Please remember to backport as well.

Richard.

> gcc/ChangeLog:
>
> 2021-11-17  Jan Hubicka  <hubicka@ucw.cz>
>
>         PR ipa/103246
>         * ipa-modref.c (read_modref_records): Fix streaminig in of every_access
>         flag.
>
> diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
> index 9ceecdd479f..c94f0589d44 100644
> --- a/gcc/ipa-modref.c
> +++ b/gcc/ipa-modref.c
> @@ -3460,10 +3460,10 @@ read_modref_records (lto_input_block *ib, struct data_in *data_in,
>           size_t every_access = streamer_read_uhwi (ib);
>           size_t naccesses = streamer_read_uhwi (ib);
>
> -         if (nolto_ref_node)
> -           nolto_ref_node->every_access = every_access;
> -         if (lto_ref_node)
> -           lto_ref_node->every_access = every_access;
> +         if (nolto_ref_node && every_access)
> +           nolto_ref_node->collapse ();
> +         if (lto_ref_node && every_access)
> +           lto_ref_node->collapse ();
>
>           for (size_t k = 0; k < naccesses; k++)
>             {

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-11-18 14:16 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-17 21:06 Fix gamess miscompare Jan Hubicka
2021-11-18 14:16 ` Richard Biener

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).