> Am 04.07.2025 um 23:19 schrieb H.J. Lu <hjl.to...@gmail.com>:
>
> On Fri, Jul 4, 2025 at 6:07 PM Richard Biener
> <richard.guent...@gmail.com> wrote:
>>
>>> On Fri, Jul 4, 2025 at 10:33 AM H.J. Lu <hjl.to...@gmail.com> wrote:
>>>
>>> On Fri, Jul 4, 2025 at 4:28 PM Richard Biener
>>> <richard.guent...@gmail.com> wrote:
>>>>
>>>> On Fri, Jul 4, 2025 at 10:21 AM H.J. Lu <hjl.to...@gmail.com> wrote:
>>>>>
>>>>> On Fri, Jul 4, 2025 at 4:10 PM H.J. Lu <hjl.to...@gmail.com> wrote:
>>>>>>
>>>>>> On Fri, Jul 4, 2025 at 4:09 PM H.J. Lu <hjl.to...@gmail.com> wrote:
>>>>>>>
>>>>>>> On Fri, Jul 4, 2025 at 4:02 PM Richard Biener
>>>>>>> <richard.guent...@gmail.com> wrote:
>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> /* If we can't trust the parm stack slot to be aligned enough for its
>>>>>>> ultimate type, don't use that slot after entry. We'll make another
>>>>>>> stack slot, if we need one. */
>>>>>>> if (stack_parm
>>>>>>> && ((GET_MODE_ALIGNMENT (data->nominal_mode) > MEM_ALIGN
>>>>>>> (stack_parm)
>>>>>>> && ((optab_handler (movmisalign_optab, data->nominal_mode)
>>>>>>> != CODE_FOR_nothing)
>>>>>>> || targetm.slow_unaligned_access (data->nominal_mode,
>>>>>>> MEM_ALIGN
>>>>>>> (stack_parm))))
>>>>>>> || (data->nominal_type
>>>>>>> && TYPE_ALIGN (data->nominal_type) > MEM_ALIGN (stack_parm)
>>>>>
>>>>> This is true.
>>>>>
>>>>>>> && MEM_ALIGN (stack_parm) < PREFERRED_STACK_BOUNDARY)))
>>>>>
>>>>> This is false as both are 16 bytes. This means that we only make a copy
>>>>> if the
>>>>> argument alignment < 16 bytes and the desired alignment > the argument
>>>>> alignment.
>>>>> It isn't the case here.
>>>>
>>>> I don't know the exact history of this code (I just was aware we have
>>>> such code), nor
>>>> the reasoning for this exception. IMO this is a correctness issue, I
>>>> could understand
>>>> < BIGGEST_ALIGNMENT or so (no possible move insn could require bigger
>>>> alignment),
>>>> but even then the GIMPLE optimizers could have taken advantage of
>>>> bigger alignment
>>>> (for example the vectorizer happily will, emitting aligned moves).
>>>>
>>>> Having a copy is less than ideal, of course. If the alignment on the
>>>> type is never
>>>> too big (see test coverage in my other mail) then we could argue to ignore
>>>> DECL_ALIGN on PARM_DECLs and instead set that to what the target thinks
>>>> in the first place when creating the decl.
>>>>
>>>
>>> Or we can change the C frontend to behave like the C++ frontend,
>>> regarding the alignment attribute. With the change at:
>>>
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120900#c10
>>>
>>> I got the following regressions:
>>>
>>> FAIL: c-c++-common/builtin-has-attribute-7.c -Wc++-compat (test for
>>> excess errors)
>>> FAIL: gcc.dg/sso-4.c (test for errors, line 18)
>>> FAIL: gcc.dg/sso-4.c (test for errors, line 19)
>>>
>>> with the change above.
>>
>> struct S { long i; long j; };
>> typedef long v2di __attribute__((vector_size(16)));
>> void foo (struct S a __attribute__((aligned(16))), long *q)
>> {
>> *(v2di *)q = *(v2di *) &a;
>> }
>>
>> is rejected for both the C and the C++ frontend with
>>
>> t.c:3:20: error: alignment may not be specified for ‘a’
>>
>> so IMO it is inconsistent to have DECL_USER_ALIGN on a PARM_DECL
>> (or DECL_ALIGN not matching that of its type).
>>
>> Richard.
>
> Regardless of how the frontend issues should be addressed, it is still
> wrong to ignore targetm.calls.function_arg_boundary, which causes
> the ICE later.
The question is whether the design is that the PARM_DECL does not need to match
the ABI (the existing caller copy workaround suggests this) or whether it
should, overriding user provided declarations (and thus possibly have the
gimplifier insert copies).
In the end your patches seem to fixup the wrong place.
But yes, there is a bug. To me it’s in the missed caller copy.
Richard
>
> --
> H.J.
