public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <richard.guenther@gmail.com>
To: Andrew MacLeod <amacleod@redhat.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>,
	"hernandez, aldy" <aldyh@redhat.com>
Subject: Re: [PATCH] PR tree-optimization/107570 - Reset SCEV after folding in VRP.
Date: Thu, 2 Feb 2023 13:22:35 +0100	[thread overview]
Message-ID: <CAFiYyc08JL2-CPxZvgbPEPAbPxknBXPWcd5r+5zQytQW5_7Xpg@mail.gmail.com> (raw)
In-Reply-To: <df275d3e-145b-c8ca-5947-db2661e84262@redhat.com>

On Wed, Feb 1, 2023 at 7:12 PM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> We can reset SCEV after we fold, then SCEVs cache shouldn't have
> anything in it when we go to remove ssa-names in remove_unreachable().
>
> We were resetting it later sometimes if we were processing the array
> bounds warning, so I removed that call and just always reset it now.
>
> Bootstraps on x86_64-pc-linux-gnu. Testing running. Assuming no
> regressions,  OK for trunk?

+
+  // SCEV needs to be reset for array bounds, and we do not wish to trigger
+  // any SCEV lookups when removing unreachable globals, so reset it here.
+  scev_reset ();

the comment suggests that SCEV queries (aka analyze_scalar_evolution)
won't return anything after a scev_reset ().  That's not true - instead what
it does is nuke the SCEV cache.  That's necessary when you
release SSA names or alter the CFG and you want to avoid followup
SCEV queries to pick up stale data.

So if remove_and_update_globals performs SCEV queries and eventually
releases SSA names you cannot remove the second call to scev_reset.

But yes, it's probably substitute_and_fold_engine::substitute_and_fold
itself that should do a

  if (scev_initialized_p ())
    scev_reset ();

possibly only in the case it released an SSA name, or removed an
edge (but that's maybe premature optimization).

Richard.

>
> Andrew

  reply	other threads:[~2023-02-02 12:22 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-01 18:11 Andrew MacLeod
2023-02-02 12:22 ` Richard Biener [this message]
2023-02-03 15:54   ` Andrew MacLeod
2023-02-03 18:47     ` Richard Biener

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=CAFiYyc08JL2-CPxZvgbPEPAbPxknBXPWcd5r+5zQytQW5_7Xpg@mail.gmail.com \
    --to=richard.guenther@gmail.com \
    --cc=aldyh@redhat.com \
    --cc=amacleod@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    /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).