From: Jeff Law <jeffreyalaw@gmail.com>
To: Richard Biener <rguenther@suse.de>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH 2/2] rtl-optimization/113255 - avoid re-associating REG_POINTER MINUS
Date: Fri, 2 Feb 2024 07:40:10 -0700 [thread overview]
Message-ID: <2bf60308-cb26-414c-a6e2-4acecf78a53f@gmail.com> (raw)
In-Reply-To: <20240201142121.B149B38582BB@sourceware.org>
On 2/1/24 07:20, Richard Biener wrote:
> The following avoids re-associating
>
> (minus:DI (reg/f:DI 119)
> (minus:DI (reg/f:DI 120)
> (reg/f:DI 114)))
>
> into
>
> (minus:DI (plus:DI (reg/f:DI 114)
> (reg/f:DI 119))
> (reg/f:DI 120))
>
> as that possibly confuses the REG_POINTER heuristics of RTL
> alias analysis. This happens to miscompile the PRs testcase
> during DSE which expands addresses via CSELIB which eventually
> simplifies what it substituted to. The original code does
> the innocent ptr - (ptr2 - ptr2'), bias a pointer by the
> difference of two other pointers.
>
> --
>
> This is what I propose for the PR for branches, I have not made much
> progress with fixing the fallout on the RTL alias analysis change
> on trunk, so this is the alternative if we decide to revert that.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu on the gcc-13
> branch, bootstrapped after reverting of the previous fix on
> x86_64-unknown-linux-gnu on trunk, testing is still ongoing there.
>
> OK? Any preference for trunk?
>
> Thanks,
> Richard.
>
> PR rtl-optimization/113255
> * simplify-rtx.cc (simplify_context::simplify_binary_operation_1):
> Do not re-associate a MINUS with a REG_POINTER op0.
Nasty little set of problems. I don't think we ever pondered that we
could have multiple REGNO_POINTER_FLAG objects in the same expression,
but clearly that can happen once you introduce a 3rd term in the expression.
I don't mind avoiding the reassociation, but it feels like we're
papering over problems in alias.cc. Conceptually it seems like if we
have two objects with REG_POINTER set, then we can't know which one is
the real base. So your patch in the PR wasn't that bad.
Alternately, just stop using REG_POINTER for alias analysis? It looks
fundamentally flawed to me in that context. In fact, one might argue
that the only legitimate use would be to indicate to the target that we
know a pointer points into an object. Some targets (the PA) need this
because x + y is not the same as y + x when used as a memory address.
If we wanted to be a bit more surgical, drop REG_POINTER from just the
MINUS handling in alias.cc?
Jeff
next parent reply other threads:[~2024-02-02 14:40 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20240201142121.B149B38582BB@sourceware.org>
2024-02-02 14:40 ` Jeff Law [this message]
2024-02-05 8:15 ` Richard Biener
2024-02-05 15:43 ` Jeff Law
2024-02-06 8:59 ` Richard Biener
2024-02-01 14:20 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=2bf60308-cb26-414c-a6e2-4acecf78a53f@gmail.com \
--to=jeffreyalaw@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=rguenther@suse.de \
/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).