From 1135a1330cf66964e516fcd3f64beeaa7a4da848 Mon Sep 17 00:00:00 2001 From: Dominik Vogt Date: Tue, 23 Aug 2016 09:20:15 +0000 Subject: [PATCH] Drop excess size used for run time allocated stack variables. * 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. gcc/ChangeLog: 2016-08-23 Dominik Vogt * explow.c (get_dynamic_stack_size): Take known alignment of stack pointer + STACK_DYNAMIC_OFFSET into account when calculating the size needed. Correct a typo in a comment. From-SVN: r239688 --- gcc/ChangeLog | 7 +++++++ gcc/explow.c | 14 ++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 8b1dc4ba051..23cf5bc33cf 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,10 @@ +2016-08-23 Dominik Vogt + + * explow.c (get_dynamic_stack_size): Take known alignment of stack + pointer + STACK_DYNAMIC_OFFSET into account when calculating the size + needed. + Correct a typo in a comment. + 2016-08-23 Dominik Vogt * config/s390/s390.md ("*andc_split"): New splitter for and with diff --git a/gcc/explow.c b/gcc/explow.c index a345690b810..f97a214f655 100644 --- a/gcc/explow.c +++ b/gcc/explow.c @@ -1224,9 +1224,15 @@ get_dynamic_stack_size (rtx *psize, unsigned size_align, example), so we must preventively align the value. We leave space in SIZE for the hole that might result from the alignment operation. */ - extra = (required_align - BITS_PER_UNIT) / BITS_PER_UNIT; - size = plus_constant (Pmode, size, extra); - size = force_operand (size, NULL_RTX); + unsigned known_align = REGNO_POINTER_ALIGN (VIRTUAL_STACK_DYNAMIC_REGNUM); + if (known_align == 0) + known_align = BITS_PER_UNIT; + if (required_align > known_align) + { + extra = (required_align - known_align) / BITS_PER_UNIT; + size = plus_constant (Pmode, size, extra); + size = force_operand (size, NULL_RTX); + } if (flag_stack_usage_info && pstack_usage_size) *pstack_usage_size += extra; @@ -1235,7 +1241,7 @@ get_dynamic_stack_size (rtx *psize, unsigned size_align, size_align = BITS_PER_UNIT; /* Round the size to a multiple of the required stack alignment. - Since the stack if presumed to be rounded before this allocation, + Since the stack is presumed to be rounded before this allocation, this will maintain the required alignment. If the stack grows downward, we could save an insn by subtracting -- 2.30.2