From: Richard Biener <richard.guenther@gmail.com>
To: Toon Moene <toon@moene.org>
Cc: Richard Biener <rguenther@suse.de>,
gcc-patches@gcc.gnu.org, jeffreyalaw@gmail.com
Subject: Re: [PATCH] rtl-optimization/113597 - recover base term for argument pointers
Date: Mon, 12 Feb 2024 09:00:15 +0100 [thread overview]
Message-ID: <CAFiYyc2nixYUDQO-Yy8f0h_4NPLR2i=srwheFYB_0Gv7HjeBVg@mail.gmail.com> (raw)
In-Reply-To: <e7d92b09-9db2-4dcb-934e-bd1d5f44cbf2@moene.org>
On Sat, Feb 10, 2024 at 10:21 PM Toon Moene <toon@moene.org> wrote:
>
> I managed to try this patch on aarch64-linux-gnu:
>
> This is the test run without your patch:
>
> https://gcc.gnu.org/pipermail/gcc-testresults/2024-February/807637.html
>
> And this is the "result" with your patch:
>
> https://gcc.gnu.org/pipermail/gcc-testresults/2024-February/807645.html
Interesting .. this looks like a miscompile of stage2. Note that the Linaro
CI was happy:
https://patchwork.sourceware.org/project/gcc/patch/20240209102649.09C543858404@sourceware.org/
Richard.
> For me, as for you, it works for x86_64-linux-gnu:
>
> https://gcc.gnu.org/pipermail/gcc-testresults/2024-February/807609.html
>
> I hope this helps.
>
> Kind regards,
> Toon Moene.
>
> On 2/9/24 11:26, Richard Biener wrote:
> > The following allows a base term to be derived from an existing
> > MEM_EXPR, notably the points-to set of a MEM_REF base. For the
> > testcase in the PR this helps RTL DSE elide stores to a stack
> > temporary. This covers pointers to NONLOCAL which can be mapped
> > to arg_base_value, helping to disambiguate against other special
> > bases (ADDRESS) as well as PARM_DECL accesses.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >
> > This is an attempt to recover some of the losses from dumbing down
> > find_base_{term,value}. I did give my ideas how to properly do
> > this during stage1 a start, I will post a short incomplete RFC series
> > later today.
> >
> > OK for trunk?
> >
> > I've included all languages in testing and also tested with -m32 but
> > details of RTL alias analysis might escape me ...
> >
> > Thanks,
> > Richard.
> >
> > PR rtl-optimization/113597
> > * alias.cc (find_base_term): Add argument for the whole mem
> > and derive a base term from its MEM_EXPR.
> > (true_dependence_1): Pass down the MEMs to find_base_term.
> > (write_dependence_p): Likewise.
> > (may_alias_p): Likewise.
> > ---
> > gcc/alias.cc | 43 ++++++++++++++++++++++++++++++++++++-------
> > 1 file changed, 36 insertions(+), 7 deletions(-)
> >
> > diff --git a/gcc/alias.cc b/gcc/alias.cc
> > index 6fad4b29d31..e33c56b0e80 100644
> > --- a/gcc/alias.cc
> > +++ b/gcc/alias.cc
> > @@ -40,6 +40,9 @@ along with GCC; see the file COPYING3. If not see
> > #include "rtl-iter.h"
> > #include "cgraph.h"
> > #include "ipa-utils.h"
> > +#include "stringpool.h"
> > +#include "value-range.h"
> > +#include "tree-ssanames.h"
> >
> > /* The aliasing API provided here solves related but different problems:
> >
> > @@ -190,6 +193,10 @@ static struct {
> > arguments, since we do not know at this level whether accesses
> > based on different arguments can alias. The ADDRESS has id 0.
> >
> > + This is solely useful to disambiguate against other ADDRESS
> > + bases as we know incoming pointers cannot point to local
> > + stack, frame or argument space.
> > +
> > 2. stack_pointer_rtx, frame_pointer_rtx, hard_frame_pointer_rtx
> > (if distinct from frame_pointer_rtx) and arg_pointer_rtx.
> > Each of these rtxes has a separate ADDRESS associated with it,
> > @@ -2113,12 +2120,34 @@ find_base_term (rtx x, vec<std::pair<cselib_val *,
> > to avoid visiting them multiple times. We unwind that changes here. */
> >
> > static rtx
> > -find_base_term (rtx x)
> > +find_base_term (rtx x, const_rtx mem = NULL_RTX)
> > {
> > auto_vec<std::pair<cselib_val *, struct elt_loc_list *>, 32> visited_vals;
> > rtx res = find_base_term (x, visited_vals);
> > for (unsigned i = 0; i < visited_vals.length (); ++i)
> > visited_vals[i].first->locs = visited_vals[i].second;
> > + if (!res && mem && MEM_EXPR (mem))
> > + {
> > + tree base = get_base_address (MEM_EXPR (mem));
> > + if (TREE_CODE (base) == PARM_DECL
> > + && DECL_RTL_SET_P (base))
> > + /* We need to look at how we expanded a PARM_DECL. It might be in
> > + the argument space (UNIQUE_BASE_VALUE_ARGP) or it might
> > + be spilled (UNIQUE_BASE_VALUE_FP/UNIQUE_BASE_VALUE_HFP). */
> > + res = find_base_term (DECL_RTL (base));
> > + else if (TREE_CODE (base) == MEM_REF
> > + && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME
> > + && SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0)))
> > + {
> > + auto pt = &SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0))->pt;
> > + if (pt->nonlocal
> > + && !pt->anything
> > + && !pt->escaped
> > + && !pt->ipa_escaped
> > + && bitmap_empty_p (pt->vars))
> > + res = arg_base_value;
> > + }
> > + }
> > return res;
> > }
> >
> > @@ -3035,13 +3064,13 @@ true_dependence_1 (const_rtx mem, machine_mode mem_mode, rtx mem_addr,
> > if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
> > return true;
> >
> > - base = find_base_term (x_addr);
> > + base = find_base_term (x_addr, x);
> > if (base && (GET_CODE (base) == LABEL_REF
> > || (GET_CODE (base) == SYMBOL_REF
> > && CONSTANT_POOL_ADDRESS_P (base))))
> > return false;
> >
> > - rtx mem_base = find_base_term (true_mem_addr);
> > + rtx mem_base = find_base_term (true_mem_addr, mem);
> > if (! base_alias_check (x_addr, base, true_mem_addr, mem_base,
> > GET_MODE (x), mem_mode))
> > return false;
> > @@ -3142,7 +3171,7 @@ write_dependence_p (const_rtx mem,
> > if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
> > return true;
> >
> > - base = find_base_term (true_mem_addr);
> > + base = find_base_term (true_mem_addr, mem);
> > if (! writep
> > && base
> > && (GET_CODE (base) == LABEL_REF
> > @@ -3150,7 +3179,7 @@ write_dependence_p (const_rtx mem,
> > && CONSTANT_POOL_ADDRESS_P (base))))
> > return false;
> >
> > - rtx x_base = find_base_term (true_x_addr);
> > + rtx x_base = find_base_term (true_x_addr, x);
> > if (! base_alias_check (true_x_addr, x_base, true_mem_addr, base,
> > GET_MODE (x), GET_MODE (mem)))
> > return false;
> > @@ -3265,8 +3294,8 @@ may_alias_p (const_rtx mem, const_rtx x)
> > if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
> > return true;
> >
> > - rtx x_base = find_base_term (x_addr);
> > - rtx mem_base = find_base_term (mem_addr);
> > + rtx x_base = find_base_term (x_addr, x);
> > + rtx mem_base = find_base_term (mem_addr, mem);
> > if (! base_alias_check (x_addr, x_base, mem_addr, mem_base,
> > GET_MODE (x), GET_MODE (mem_addr)))
> > return false;
>
> --
> Toon Moene - e-mail: toon@moene.org - phone: +31 346 214290
> Saturnushof 14, 3738 XG Maartensdijk, The Netherlands
>
next prev parent reply other threads:[~2024-02-12 8:00 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20240209102637.6BB1D3858438@sourceware.org>
2024-02-10 21:20 ` Toon Moene
2024-02-12 8:00 ` Richard Biener [this message]
2024-02-12 15:26 ` Christophe Lyon
[not found] <65c5fdce.5d0a0220.b4fa7.11a6SMTPIN_ADDED_MISSING@mx.google.com>
2024-03-03 23:58 ` Jeff Law
2024-03-04 8:05 ` Richard Biener
2024-02-09 10:26 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='CAFiYyc2nixYUDQO-Yy8f0h_4NPLR2i=srwheFYB_0Gv7HjeBVg@mail.gmail.com' \
--to=richard.guenther@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jeffreyalaw@gmail.com \
--cc=rguenther@suse.de \
--cc=toon@moene.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).