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. -- H.J.
