Re: [PATCH 2/2][v4] Drop excess size used for run time allocated stack variables.

[Dominik Vogt <vogt at linux dot vnet dot ibm dot com>]

Finally a patch that works and is simple.  Bootstrapped and
regression tested on s390, s390x biarch and x86_64.  The new patch
exploits the known alignment of (stack pointer +
STACK_DYNAMIC_OFFSET) as described earlier (see below).  I think
that is the right way to get rid of the extra allocation.  It
took a long time to understand the problem.

As the patch triggers a bug in the fortran compiler, the
der_type.f90 test case may fail on some targets if this patch is
used without the fortran fix that I've posted in another thread.

(The patch also contains a fix for a typo in a comment in the
patched function.)

See ChangeLog for a full description of the new patch.

Since the patch is all new, we're not going to commit it without a
new OK.

> Actually I was goind to abandon the patch in its current state.
> :-)  We talked about it internally and concluded that the problem
> is really this:
> 
>  * get_dynamic_stack_size is passed a SIZE of a data block (which
>    is allocated elsewhere), the SIZE_ALIGN of the SIZE (i.e. the
>    alignment of the underlying memory units (e.g. 32 bytes split
>    into 4 times 8 bytes = 64 bit alignment) and the
>    REQUIRED_ALIGN of the data portion of the allocated memory.
>  * Assuming the function is called with SIZE = 2, SIZE_ALIGN = 8
>    and REQUIRED_ALIGN = 64 it first adds 7 bytes to SIZE -> 9.
>    This is what is needed to have two bytes 8-byte-aligned at some
>    memory location without any known alignment.
>  * Finally round_push is called to round up SIZE to a multiple of
>    the stack slot size.
> 
> The key to understanding this is that the function assumes that
> STACK_DYNMAIC_OFFSET is completely unknown at the time its called
> and therefore it does not make assumptions about the alignment of
> STACKPOINTER + STACK_DYNMAIC_OFFSET.  The latest patch simply
> hard-codes that SP + SDO is supposed to be aligned to at least
> stack slot size (and does that in a very complicated way).  Since
> there is no guarantee that this is the case on all targets, the
> patch is broken.  It may miscalculate a SIZE that is too small in
> some cases.
> 
> However, on many targets there is some guarantee about the
> alignment of SP + SDO even if the actual value of SDO is unknown.
> On s390x it's always 8-byte-aligned (stack slot size).  So the
> right fix should be to add knowledge about the target's guaranteed
> alignment of SP + SDO to the function.  I'm right now testing a
> much simpler patch that uses
> REGNO_POINTER_ALIGN(VIRTUAL_STACK_DYNAMIC_REGNUM) as the
> alignment.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

Attachment: 0001-v4-ChangeLog
Description: Text document

Attachment: 0001-v4-Reduce-size-allocated-for-run-time-allocated-stack-v.patch
Description: Text document