On 6/2/21 7:01 AM, Richard Biener wrote:
> On Wed, Jun 2, 2021 at 1:15 PM Pat Haugen <pthau...@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 <pthau...@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.
__builtin_return_address creates it's own copy of the link reg to a pseudo upon
function entry. It doesn't appear to try and "reuse" any LR copy/save location
that might be generated via the prolog code. References to
__builtin_return_address will then refer to that pseudo. So I don't see any
connection between the prolog save code and __builtin_return_address.
-Pat
