Re: [PATCH 1/3] rtl: allow forming subregs of already unaligned mems [PR102125]

[Richard Earnshaw <Richard.Earnshaw@foss.arm.com>]

On 06/09/2021 12:13, Richard Biener wrote:
> On Mon, Sep 6, 2021 at 1:08 PM Richard Earnshaw
> <Richard.Earnshaw@foss.arm.com> wrote:
>>
>>
>>
>> On 06/09/2021 11:58, Richard Biener via Gcc-patches wrote:
>>> On Mon, Sep 6, 2021 at 12:40 PM Richard Earnshaw <rearnsha@arm.com> wrote:
>>>>
>>>>
>>>> GCC was recently changed to prevent simplify_subreg from simplifying
>>>> a subreg of a mem when the mode of the new mem would have stricter alignment
>>>> constraints than the inner mem already has when the target requires
>>>> STRICT_ALIGNMENT.
>>>>
>>>> However, such targets may have specialist patterns that can handle
>>>> unaligned accesses and this restriction turns out to be unduly restrictive.
>>>> So limit this restriction to only apply when the inner mem is naturally
>>>> aligned to the inner mode.
>>>
>>> Hmm, I think this can end up either generating wrong code or
>>> recog fails.  The specific combination of alignment and mode of 'op'
>>> has been validated to be supported, replacing the mode with sth
>>> else would need re-validation of the combination.  I'm not sure
>>> we can for example just query movmisalign support here and
>>> hope for LRA to reload the mem with that.
>>>
>>> So - where do you run into this?  Is it possible to catch the
>>> situation on a higher level where more context as in the whole insn
>>> is visible?
>>
>> I ran into it with patch 2 of this series when calling gen_highpart on a
>> misaligned mem.  IIRC gen_highpart would end up returning (subreg:SI
>> (mem:DI (addr [A8])) 4), while gen_lowpart would simplify the operation
>> to (mem:SI (addr [A8])) as expected.
>>
>> (subreg:SI (mem:DI (addr [A8])) 4) is really problematic, because it's
>> not a memory_operand (from the manual: it will get reloaded into a
>> register later on).  But that's no good here, I don't want this
>> reloading into a wide register later, I need it to be narrowed to the
>> component part now.
> 
> So maybe calling gen_highpart is not what you want then?
> adjust_address is IIRC what one uses to offset a MEM and change
> its mode.

It was based on looking at the patch for PR 100106 
(r12-163-c33db31d9ad9).  That patch added the MEM_ALIGN constraint when 
previously there was none here and my call would have been simplified. 
Are you saying that GCC was always wrong in this respect?  All I've done 
was to tightened the check that Bernd added.

R.

> 
> Richard.
> 
>>
>> R.
>>
>>>
>>> Thanks,
>>> Richard.
>>>
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>>           PR target/102125
>>>>           * simplify-rtx.c (simplify_context::simplify_subreg): Allow
>>>>           simplifying (subreg (mem())) when the inner mem is already
>>>>           misaligned for its type.
>>>> ---
>>>>    gcc/simplify-rtx.c | 6 +++++-
>>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>>