On Tue, Jul 8, 2025 at 3:25 PM Jason Merrill <ja...@redhat.com> wrote:
>
> On 7/8/25 4:35 AM, Richard Biener wrote:
> > On Mon, Jul 7, 2025 at 11:33 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> >>
> >> On Tue, Jul 8, 2025 at 5:02 AM H.J. Lu <hjl.to...@gmail.com> wrote:
> >>>
> >>> On Mon, Jul 7, 2025 at 11:08 PM Jason Merrill <ja...@redhat.com> wrote:
> >>>>
> >>>> On 7/1/25 5:36 PM, H.J. Lu wrote:
> >>>>> On Tue, Jul 1, 2025 at 9:37 PM Jason Merrill <ja...@redhat.com> wrote:
> >>>>>>
> >>>>>> On 6/30/25 7:03 PM, H.J. Lu wrote:
> >>>>>>> On Mon, Jun 30, 2025 at 10:36 PM Jason Merrill <ja...@redhat.com>
> >>>>>>> wrote:
> >>>>>>>>
> >>>>>>>> On 6/28/25 7:00 AM, H.J. Lu wrote:
> >>>>>>>>> Since a backend may ignore user type alignment for arguments passed
> >>>>>>>>> on
> >>>>>>>>> stack, check backend for argument alignment on stack when evaluating
> >>>>>>>>> __alignof.
> >>>>>>>>
> >>>>>>>> I assume that's reflected in DECL_ALIGN, so could we just add
> >>>>>>>> PARM_DECL to
> >>>>>>>
> >>>>>>> No. targetm.calls.function_arg_boundary may have special handling
> >>>>>>> for it.
> >>>>>>
> >>>>>> Why wouldn't we adjust DECL_ALIGN of the PARM_DECL to reflect the
> >>>>>> actual
> >>>>>> alignment of the argument? Are you saying it could be different from
> >>>>>> one call to another?
> >>>>>
> >>>>> Function argument alignment is different from other places in memory if
> >>>>> the main variant type alignment is different:
> >>>>
> >>>> Yes, I understand that function parameter alignment can be different
> >>>> from other objects of that type.
> >>>>
> >>>> But since we have a PARM_DECL to represent that particular function
> >>>> parameter, it seems natural to represent that difference in the
> >>>> DECL_ALIGN of the PARM_DECL. If you don't, its DECL_ALIGN is wrong.
> >>>>
> >>>
> >>> __alignof returns TYPE_ALIGN, not DECL_ALIGN. For PARM_DECL,
> >>> TYPE_ALIGN may not be the same as DECL_ALIGN.
> >>>
> >>
> >> How about this patch?
>
> > @@ -4097,6 +4097,9 @@ c_alignof_expr (location_t loc, tree expr)
> > }
> > return c_alignof (loc, TREE_TYPE (TREE_TYPE (best)));
> > }
> > + /* For PARM_DECL, DECL_ALIGN may be different from TYPE_ALIGN. */
> > + else if (TREE_CODE (expr) == PARM_DECL)
> > + return size_int (DECL_ALIGN (expr) / BITS_PER_UNIT);
>
> I was suggesting that you could add PARM_DECL to this case at the top of
> the function:
>
> > else if (VAR_OR_FUNCTION_DECL_P (expr))
> > t = size_int (DECL_ALIGN_UNIT (expr));
>
> >> Since a backend may ignore type alignment for arguments passed on stack,
> >> call targetm.calls.function_arg_boundary to set DECL_ALIGN for PARM_DECL
> >> and change __alignof to return DECL_ALIGN, instead of TYPE_ALIGN, for
> >> PARM_DECL.
> >
> > I don't think this will work out correctness-wise. You'd have to patch up
> > all places. Also we might turn a reference to the PARM_DECL into
> > a dereference of its address. So we rely on the fact that TYPE_ALIGN
> > is always more conservative than DECL_ALIGN which is not the case
> > you are caring about.
>
> Hmm, but that's not always true with attribute packed? Though that only
> applies to fields, so I suppose extending DECL_PACKED handling to
> PARM_DECL might be more work than copying the underaligned passed
> argument into a local variable as you suggest.
I'm not saying we have no cases contrary to how it should work, but
I'm insisting
we don't add more of them. A decl can be hidden from the observer in various
ways (trivially through indirection), when only the type remains its alignment
has to be conservative.
Note that __attribute__((aligned)) is rejected on PARM_DECLs,
__attribute__((packed))
is ignored.
That said, when you "fix" DECL_ALIGN of PARM_DECLs according to what the target
thinks it will still make __alignof/__alignas behave wrong, no? Aka,
template <class T>
void foo (T x)
{
static_assert (__alignof (T) == __alignof (x));
}
should work for all types?
Richard.
>
> > So no, I don't think this is good design. Get the missed copy working
> > instead.
> >
> > Richard.
> >
> >> gcc/
> >>
> >> PR target/120839
> >> * stor-layout.cc (do_type_align): Call
> >> targetm.calls.function_arg_boundary to set DECL_ALIGN for
> >> PARM_DECL.
> >>
> >> gcc/c-family/
> >>
> >> PR target/120839
> >> * c-common.cc (c_alignof_expr): Return DECL_ALIGN for PARM_DECL.
> >>
> >> gcc/testsuite/
> >>
> >> PR target/120839
> >> * gcc.target/i386/pr120839-1.c: New test.
> >> * gcc.target/i386/pr120839-2.c: Likewise.
> >>
> >> --
> >> H.J.
> >
>
