From: Jeff Law <jeffreyalaw@gmail.com>
To: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH v2] cselib: add function to check if SET is redundant [PR106187]
Date: Sat, 30 Jul 2022 13:57:18 -0600 [thread overview]
Message-ID: <9e9b7ec5-4c10-115d-8b6d-a7a07bfd72be@gmail.com> (raw)
In-Reply-To: <cf3b683e-ac3d-a6f3-f98e-f39a66fcb16f@arm.com>
On 7/29/2022 7:52 AM, Richard Earnshaw via Gcc-patches wrote:
> A SET operation that writes memory may have the same value as an
> earlier store but if the alias sets of the new and earlier store do
> not conflict then the set is not truly redundant. This can happen,
> for example, if objects of different types share a stack slot.
>
> To fix this we define a new function in cselib that first checks for
> equality and if that is successful then finds the earlier store in the
> value history and checks the alias sets.
>
> The routine is used in two places elsewhere in the compiler. Firstly
> in cfgcleanup and secondly in postreload.
>
> gcc/ChangeLog:
> * alias.h (mems_same_for_tbaa_p): Declare.
> * alias.cc (mems_same_for_tbaa_p): New function.
> * dse.cc (record_store): Use it instead of open-coding
> alias check.
> * cselib.h (cselib_redundant_set_p): Declare.
> * cselib.cc: Include alias.h
> (cselib_redundant_set_p): New function.
> * cfgcleanup.cc: (mark_effect): Use cselib_redundant_set_p instead
> of rtx_equal_for_cselib_p.
> * postreload.c (reload_cse_simplify): Use cselib_redundant_set_p.
> (reload_cse_noop_set_p): Delete.
Seems quite reasonable. The only question I would have would be
whether or not you considered including the aliasing info into the
hashing used by cselib. You'd probably still need the bulk of this
patch as well since we could presumably still get a hash conflict with
two stores of the same value to the same location, but with different
alias sets (it's just much less likely), so perhaps it doesn't really
buy us anything.
Ideally this would include a testcase. You might be able to turn that
non-executawble reduced case into something useful by scanning the
post-reload dumps.
Jeff
next prev parent reply other threads:[~2022-07-30 19:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-29 13:52 Richard Earnshaw
2022-07-30 19:57 ` Jeff Law [this message]
2022-08-01 10:38 ` Richard Earnshaw
2022-08-02 16:06 ` Richard Earnshaw
2022-08-02 23:36 ` Jeff Law
2022-08-03 9:09 ` Richard Earnshaw
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=9e9b7ec5-4c10-115d-8b6d-a7a07bfd72be@gmail.com \
--to=jeffreyalaw@gmail.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).