This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch] PR 65315 - Fix alignment of local variables
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Steve Ellcey <sellcey at imgtec dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 5 Mar 2015 09:36:26 +0100
- Subject: Re: [Patch] PR 65315 - Fix alignment of local variables
- Authentication-results: sourceware.org; auth=none
- References: <2db74cc6-b63d-4f8b-aa37-525b51b0b34c at BAMAIL02 dot ba dot imgtec dot org>
On Wed, Mar 4, 2015 at 8:50 PM, Steve Ellcey <sellcey@imgtec.com> wrote:
> While examining some MIPS code I noticed that GCC did not seem to be
> fully honoring the aligned attribute on some local variables. I submitted
> PR middle-end/65315 to record the bug and I think I now understand it and
> have a fix. The problem was that expand_stack_vars seemed to think that
> the first entry in stack_vars_sorted would have the largest alignment but
> while all the variables that had alignment greater then
> MAX_SUPPORTED_STACK_ALIGNMENT would come before all variables whose
> alignment was less than MAX_SUPPORTED_STACK_ALIGNMENT, within the variables
> with the alignment greater than MAX_SUPPORTED_STACK_ALIGNMENT, they
> were sorted by size, not by alignment.
>
> So my fix was to update large_align in expand_stack_vars if needed.
>
> I have verified the fix on the MIPS test case in PR 65315 and am doing a
> regression test now. OK to checkin if there are no regressions?
It looks like large_align vars are dynamically allocated and thus they
should be sorted as sizeof (void *) I suppose.
Do you have a testcase?
Ok.
Thanks,
Richard.
Richad.
> I wasn't sure how to create a generic test case, I was checking the
> alignment on MIPS by hand by looking for the shift-right/shift-left
> instructions that create an aligned pointer but that obviously doesn't
> work on other architectures.
>
> Steve Ellcey
> sellcey@imgtec.com
>
>
> 2015-03-04 Steve Ellcey <sellcey@imgtec.com>
>
> PR middle-end/65315
> * cfgexpand.c (expand_stack_vars): Update large_align to maximum
> needed alignment.
>
>
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index 7dfe1f6..569cd0d 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -973,6 +973,13 @@ expand_stack_vars (bool (*pred) (size_t), struct stack_vars_data *data)
> i = stack_vars_sorted[si];
> alignb = stack_vars[i].alignb;
>
> + /* All "large" alignment decls come before all "small" alignment
> + decls, but "large" alignment decls are not sorted based on
> + their alignment. Increase large_align to track the largest
> + required alignment. */
> + if ((alignb * BITS_PER_UNIT) > large_align)
> + large_align = alignb * BITS_PER_UNIT;
> +
> /* Stop when we get to the first decl with "small" alignment. */
> if (alignb * BITS_PER_UNIT <= MAX_SUPPORTED_STACK_ALIGNMENT)
> break;