Richard Biener <[email protected]> writes:
> On Thu, 26 Mar 2026, Richard Biener wrote:
>
>> On Thu, 26 Mar 2026, Richard Biener wrote:
>> 
>> > On Thu, 26 Mar 2026, Richard Sandiford wrote:
>> > 
>> > > Richard Biener <[email protected]> writes:
>> > > > The following fixes a confusion seein in the x86 backend by
>> > > > assign_parm_adjust_stack_rtl failing to trigger a local stack copy
>> > > > for an incoming stack parameter that is not aligned according to
>> > > > its type.  The condition was introduced in r0-64961-gbfc45551d5ace4
>> > > > but there is the MEM_ALIGN (stack_parm) < PREFERRED_STACK_BOUNDARY
>> > > > condition not triggering for the case in question where both
>> > > > MEM_ALIGN and PREFERRED_STACK_BOUNDARY are 128.  I cannot really
>> > > > make sense of the condition - taking MEM_ALIGN (stack_parm)
>> > > > as the ABI mandated alignment of the argument stack slot means
>> > > > that is a lower bound on the incoming stack boundary.  But
>> > > > PREFERRED_STACK_BOUNDARY is not the actual boundary and relating
>> > > > it to a minimum known boundary to somehow infer known bigger
>> > > > alignment of the incoming parameter looks odd.
>> > > 
>> > > I have no memory of this patch, but my reading is that
>> > > PREFERRED_STACK_BOUNDARY is being used as the maximum attainable
>> > > alignment.  In other words, there is no point discarding the old
>> > > underaligned stack slot if the new one wouldn't guarantee a
>> > > greater alignment.  According to this assumption, GCC's stack
>> > > allocation code wouldn't preserve an alignment greater than
>> > > PREFERRED_STACK_BOUNDARY even if the incoming stack pointer
>> > > happened to be more aligned.
>> > > 
>> > > However, this predates the dynamic stack realignment code
>> > > (r0-88648-g2e3f842fe69ce4), so perhaps we should skip the
>> > > PREFERRED_STACK_BOUNDARY check on targets that use that?
>> > > 
>> > > E.g. on aarch64, if a type claims to have 32-byte alignment,
>> > > GCC won't try to provide that alignment for arguments passed
>> > > on the stack, whereas x86 will.  https://godbolt.org/z/eWjfcoPbb
>> > > is a not very good example.  (Not very good because the parameter is
>> > > passed by indirect reference on aarch64.  I think the point still
>> > > stands though.)
>> > 
>> > On x86 it doesn't seem to happen always (as in the PRs testcase).
>> > 
>> > But I wonder if when the ABI does not guarantee sufficient alignment
>> > of the argument stack slot, and the target does not provide a way
>> > to dynamically increase stack alignment to make the copy destination
>> > properly aligned - what should we do?  Don't we want to sorry ()
>> > in that case?  Because we have happily assumed the PARM_DECLs
>> > alignment as written.
>> > 
>> > I had assumed that dynamic stack re-alignment code works everywhere
>> > (where alloca works).
>> > 
>> > So you're suggesting to replace PREFERRED_STACK_BOUNDARY with
>> > MAX_SUPPORTED_STACK_ALIGNMENT?  And <= here.
>> > 
>> > > 
>> > > Changing < to <= doesn't look right for other targets though.
>> > > It's likely to lead to pointless copying.
>> > 
>> > Why?  If the stack can be aligned to PREFERRED_STACK_BOUNDARY
>> > but the ABI misaligns the incoming stack slot we can very well
>> > create a suitably aligned stack slot?  So in fact the < is what
>> > is odd in the first place?  Shouldn't it be testing
>> > STACK_BOUNDARY and not PREFERRED_STACK_BOUNDARY?
>
> Ah, of course.  But shouldn't this test
> TYPE_ALIGN (data->nominal_type) <= PREFERRED_STACK_BOUNDARY?  Even
> if MEM_ALIGN (stack_parm) is lees than PREFERRED_STACK_BOUNDARY,
> what's the point in copying when we still cannot get
> TYPE_ALIGN (data->nominal_type) alignment?

There's probably value in having an alignment of 32 guarantee at least
as much alignment as an alignment of 16, even if we can't go all the way.
(But I'll respond to the "can't go all the way" bit in the v2 patch.)

Richard

Reply via email to