[PATCH, rs6000] Fix alias set of link reg save MEM

Richard Biener richard.guenther@gmail.com
Wed Jun 2 12:01:34 GMT 2021


On Wed, Jun 2, 2021 at 1:15 PM Pat Haugen <pthaugen@linux.ibm.com> wrote:
>
> On 6/2/21 1:51 AM, Richard Biener wrote:
> > On Tue, Jun 1, 2021 at 10:37 PM Pat Haugen via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >> Make sure link reg save MEM has frame alias set, to match other link reg
> >> save/restore code.
> >>
> >> Bootstrap/regtest on powerpc64/powerpc64le with no new regressions. Ok for
> >> trunk?
> >>
> >> -Pat
> >>
> >>
> >> 2021-06-01  Pat Haugen  <pthaugen@linux.ibm.com>
> >>
> >> gcc/ChangeLog:
> >>
> >>         * config/rs6000/rs6000-logue.c (rs6000_emit_prologue): Use
> >>         gen_frame_store.
> >>
> >>
> >>
> >> diff --git a/gcc/config/rs6000/rs6000-logue.c b/gcc/config/rs6000/rs6000-logue.c
> >> index 13c00e740d6..07337c4836a 100644
> >> --- a/gcc/config/rs6000/rs6000-logue.c
> >> +++ b/gcc/config/rs6000/rs6000-logue.c
> >> @@ -3257,7 +3257,7 @@ rs6000_emit_prologue (void)
> >>    if (!WORLD_SAVE_P (info) && info->lr_save_p
> >>        && !cfun->machine->lr_is_wrapped_separately)
> >>      {
> >> -      rtx addr, reg, mem;
> >> +      rtx reg;
> >>
> >>        reg = gen_rtx_REG (Pmode, 0);
> >>        START_USE (0);
> >> @@ -3267,13 +3267,8 @@ rs6000_emit_prologue (void)
> >>        if (!(strategy & (SAVE_NOINLINE_GPRS_SAVES_LR
> >>                         | SAVE_NOINLINE_FPRS_SAVES_LR)))
> >>         {
> >> -         addr = gen_rtx_PLUS (Pmode, frame_reg_rtx,
> >> -                              GEN_INT (info->lr_save_offset + frame_off));
> >> -         mem = gen_rtx_MEM (Pmode, addr);
> >> -         /* This should not be of rs6000_sr_alias_set, because of
> >> -            __builtin_return_address.  */
> >
> > I can't figure what this comment meant - did you?  Note the old code
> > looks like it would end up with alias-set zero and thus more conservative
> > than with using frame-alias-set so this is an optimization?
>
> No, I couldn't figure out the reasoning for the comment/code either. It's been that way since it was introduced in March 2000 as part of the “Merge in changes from newppc-branch.” patch. All other places where we save/restore the link reg use a MEM with frame-alias-set. This change is an optimization as you suspect in that it allows us to schedule non-aliased loads above the link reg store (which couldn't happen before due to use of alias-set zero).

So did you check the RTL (and alias-sets) produced by
__builtin_return_address?  Test coverage might
be low here and w/o scheduling opportunities to break things.

Richard.

> -Pat


More information about the Gcc-patches mailing list