> I pondered that as a direction, but was scared off by the overall
> fragility of this code when I looked back through the old BZs.  I
> figured cleanup preserving existing behavior was the first step.

Setting aside the flag_split_stack stuff, the must_align logic is clear enough 
and quite localized.

> We can go the other way if you prefer.   It just makes reasoning about
> how this code is supposed to work harder.

On second thoughts, there might be a real issue with STACK_DYNAMIC_OFFSET that 
is papered over by the STACK_POINTER_OFFSET glitch.  The big comment explains 
why, if STACK_DYNAMIC_OFFSET is defined, we need to preventively align, and 
that it's defined if ACCUMULATE_OUTGOING_ARGS is activated.  That's not true 
for STACK_DYNAMIC_OFFSET overall, but that's indeed true in function.c where 
STACK_DYNAMIC_OFFSET is forcibly defined.

Given that ACCUMULATE_OUTGOING_ARGS is activated by default on most mainstream 
platforms, it probably makes sense to always preventively align.  From there 
on, I think that what would be left to discuss is the amount of alignment we 
can start from (extra_align in the current code); maybe BITS_PER_UNIT is also 
the only sensible value in the end.

So I think that your patch is OK if you rescue the main comment, for example:

  /* We will need to ensure that the address we return is aligned to
     REQUIRED_ALIGN.  At this point in the compilation, we don't always
     know the final value of the STACK_DYNAMIC_OFFSET used in function.c
     (it might depend on the size of the outgoing parameter lists, for
     example), so we must preventively align the value.  We leave space
     in SIZE for the hole that might result from the alignment operation.  */

-- 
Eric Botcazou

Reply via email to