On Wed, Mar 4, 2015 at 8:50 PM, Steve Ellcey <sell...@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 > sell...@imgtec.com > > > 2015-03-04 Steve Ellcey <sell...@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;
