Re: [PATCH v2] [PR100106] Reject unaligned subregs when strict alignment is required

[Jeff Law <jeffreyalaw@gmail.com>]

On 5/5/2022 8:41 PM, Alexandre Oliva via Gcc-patches wrote:
> On May  5, 2022, Segher Boessenkool <segher@kernel.crashing.org> wrote:
>
>> On Thu, May 05, 2022 at 03:52:01AM -0300, Alexandre Oliva wrote:
>>> +  else if (reg && MEM_P (reg)
>>> +	   && STRICT_ALIGNMENT && MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (omode))
>>> +    return false;
>> Please fix the line breaks?  Either do a break before every &&, or put
>> as many things as possible on one line?
> I was going for conceptual grouping of alignment-related subexprs,
> but I don't care enough to fight for it.
>
>> Note that you should never have paradoxical subregs of mem on rs6000 or
>> any other target with INSN_SCHEDULING.
> Great, that alleviates some of my concerns about overreaching in this patch.
>
>>> +#include "../../gcc.c-torture/compile/pr100106.c"
>> It is better to copy the 11 lines of code.
> 'k
>
>> Please comment what the ilp32 is for (namely, the -mcpu= will barf
>> without it)..
> Ack
>
>> The testcase is okay with those changes, thanks!
> Thanks.  Here's the revised patch.
>
> I'm now testing on several platforms a follow-up patch that introduces
> TARGET_ALLOW_SUBREG_OF_MEM.
>
>
> [PR100106] Reject unaligned subregs when strict alignment is required
>
> From: Alexandre Oliva <oliva@adacore.com>
>
> The testcase for pr100106, compiled with optimization for 32-bit
> powerpc -mcpu=604 with -mstrict-align expands the initialization of a
> union from a float _Complex value into a load from an SCmode
> constant pool entry, aligned to 4 bytes, into a DImode pseudo,
> requiring 8-byte alignment.
>
> The patch that introduced the testcase modified simplify_subreg to
> avoid changing the MEM to outermode, but simplify_gen_subreg still
> creates a SUBREG or a MEM that would require stricter alignment than
> MEM's, and lra_constraints appears to get confused by that, repeatedly
> creating unsatisfiable reloads for the SUBREG until it exceeds the
> insn count.
>
> Avoiding the unaligned SUBREG, expand splits the DImode dest into
> SUBREGs and loads each SImode word of the constant pool with the
> proper alignment.
>
>
> for  gcc/ChangeLog
>
> 	PR target/100106
> 	* emit-rtl.cc (validate_subreg): Reject a SUBREG of a MEM that
> 	requires stricter alignment than MEM's.
>
> for  gcc/testsuite/ChangeLog
>
> 	PR target/100106
> 	* gcc.target/powerpc/pr100106-sa.c: New.
OK.
jeff