> 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
