On 03/22/2018 05:56 AM, Renlin Li wrote: > Hi all, > > As described in PR84877. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84877 > The local copy of parameter on stack is not aligned. > > For BLKmode paramters, a local copy on the stack will be saved. > There are three cases: > 1) arguments passed partially on the stack, partially via registers. > 2) arguments passed fully on the stack. > 3) arguments passed via registers. > > After the change here, in all three cases, the stack slot for the local > parameter copy is aligned by the data type.Presumably this is only for named > arguments. If we have to deal with stdarg/varargs there's a number of additional complications we'd need to worry about.
> > The stack slot is the DECL_RTL of the parameter. All the references > thereafter in the function will refer to this RTL. OK. This implies we're dealing strictly with named arguments. > > > To populate the local copy on the stack, > For case 1) and 2), there are operations to move data from the caller's stack > (from incoming rtl) into callee's stack. > > For case 3), the registers are directly saved into the stack slot. > > In all cases, the destination address is properly aligned. > But for case 1) and case 2), the source address is not aligned by the type. > It is defined by the PCS how the arguments are prepared. I'm not 100% sure the destination is always aligned. I vaguely recall the PA being an oddball on this kind of stuff. > > The block move operation is fulfilled by emit_block_move (). As far as I can > see, Yes. But we may have had to flush argument registers to memory prior to using emit_block_move. And the flushing operation can be odd because of things like alignment, padding, etc. The PA in particular was an oddball here, but I don't remember the precise details. > > it will use the smaller alignment of source and destination. > This looks fine as long as we don't use instructions which requires a strict > larger alignment than the address actually has. Right. > > > Here, it only changes receiving parameters. > The function assign_stack_local_1 will be called in various places. > Usually, the caller will constraint the ALIGN parameter. For example via > STACK_SLOT_ALIGNMENT macro. > > assign_parm_setup_block will call assign_stack_local () with alignment from > the parameter type which in this case could be > > larger than MAX_SUPPORTED_STACK_ALIGNMENT. > > The alignment operation for parameter copy on the stack is similar to stack > vars. > > First, enough space is reserved on the stack. The size is fixed at compile > time. > > Instructions are emitted to dynamically get an aligned address at runtime > within this piece of memory. At least that's how it's supposed to work. I have some concerns about the existing dynamic alignment bits independent of your change. > > > This will unavoidably increase the usage of stack. However, it really depends > on > > how many over-aligned parameters are passed by value. It's relatively rare in my experience, so I wouldn't let this get in the way. > > x86-linux, arm-none-eabi, aarch64-one-elf regression test Okay. > linux-armhf bootstrap Okay. > > I assume there are other targets which will be affected by the change. > But I don't have environment to test. I don't think my tester will help much here as over-aligned parameters are relatively rare and likely not triggered by bootstraps. > > Okay the commit? > > > Regards, > Renlin > > gcc/ > > 2018-03-22 Renlin Li <renlin...@arm.com> > > PR middle-end/84877 > * explow.h (get_dynamic_stack_size): Declare it as external. > * explow.c (record_new_stack_level): Remove function static attribute. > * function.c (assign_stack_local_1): Dynamically align the stack slot > addr for parameter copy on the stack. > > gcc/testsuite/ > > 2018-03-22 Renlin Li <renlin...@arm.com> > > PR middle-end/84877 > * gcc.dg/pr84877.c: New. OK. Certainly keep an eye out for issues on other targets. Jeff
