Re: [PATCH] Improve alloca alignment

2017-12-19 Thread Jeff Law
On 12/13/2017 04:17 PM, Eric Botcazou wrote: >> No clue, but ISTM that it should. Eric, can you try that and see if it >> addresses these problems? I'd really like to get this wrapped up, but I >> don't have access to any sparc systems to test it myself. > > Yes, the INIT_EXPANDERS trick works

Re: [PATCH] Improve alloca alignment

2017-12-13 Thread Eric Botcazou
> No clue, but ISTM that it should. Eric, can you try that and see if it > addresses these problems? I'd really like to get this wrapped up, but I > don't have access to any sparc systems to test it myself. Yes, the INIT_EXPANDERS trick works for SPARC (but this has nothing to do with

Re: [PATCH] Improve alloca alignment

2017-10-26 Thread Richard Biener
On Thu, Oct 26, 2017 at 4:55 PM, Jeff Law wrote: > On 10/17/2017 06:04 AM, Wilco Dijkstra wrote: >> Wilco Dijkstra wrote: >>> >>> Yes STACK_BOUNDARY applies to virtual_stack_dynamic_rtx and all other >>> virtual frame registers. It appears it's main purpose is to enable alignment

Re: [PATCH] Improve alloca alignment

2017-10-26 Thread Jeff Law
On 10/17/2017 06:04 AM, Wilco Dijkstra wrote: > Wilco Dijkstra wrote: >> >> Yes STACK_BOUNDARY applies to virtual_stack_dynamic_rtx and all other >> virtual frame registers. It appears it's main purpose is to enable alignment >> optimizations since PREFERRED_STACK_BOUNDARY is used to align >>

Re: [PATCH] Improve alloca alignment

2017-10-26 Thread Jeff Law
On 10/05/2017 03:16 AM, Richard Biener wrote: > On Thu, Oct 5, 2017 at 1:07 AM, Jeff Law wrote: >> On 10/04/2017 08:53 AM, Eric Botcazou wrote: This seems like a SPARC target problem to me -- essentially it's claiming a higher STACK_BOUNDARY than it really has. >>> >>>

Re: [PATCH] Improve alloca alignment

2017-10-17 Thread Wilco Dijkstra
Wilco Dijkstra wrote: > > Yes STACK_BOUNDARY applies to virtual_stack_dynamic_rtx and all other > virtual frame registers. It appears it's main purpose is to enable alignment > optimizations since PREFERRED_STACK_BOUNDARY is used to align > local and outgoing argument area etc. So if you don't

Re: [PATCH] Improve alloca alignment

2017-10-05 Thread Wilco Dijkstra
Richard Biener wrote: > On Thu, Oct 5, 2017 at 1:07 AM, Jeff Law wrote: > > On 10/04/2017 08:53 AM, Eric Botcazou wrote: This seems like a SPARC target problem to me -- essentially it's claiming a higher STACK_BOUNDARY than it really has. >>> >>> No, it is not, I can

Re: [PATCH] Improve alloca alignment

2017-10-05 Thread Richard Biener
On Thu, Oct 5, 2017 at 1:07 AM, Jeff Law wrote: > On 10/04/2017 08:53 AM, Eric Botcazou wrote: >>> This seems like a SPARC target problem to me -- essentially it's >>> claiming a higher STACK_BOUNDARY than it really has. >> >> No, it is not, I can guarantee you that the stack

Re: [PATCH] Improve alloca alignment

2017-10-04 Thread Jeff Law
On 10/04/2017 08:53 AM, Eric Botcazou wrote: >> This seems like a SPARC target problem to me -- essentially it's >> claiming a higher STACK_BOUNDARY than it really has. > > No, it is not, I can guarantee you that the stack pointer is always aligned > to > 64-bit boundaries on SPARC, otherwise

Re: [PATCH] Improve alloca alignment

2017-10-04 Thread Eric Botcazou
> This seems like a SPARC target problem to me -- essentially it's > claiming a higher STACK_BOUNDARY than it really has. No, it is not, I can guarantee you that the stack pointer is always aligned to 64-bit boundaries on SPARC, otherwise all hell would break loose... > Presumably there's a

Re: [PATCH] Improve alloca alignment

2017-09-11 Thread Wilco Dijkstra
Jeff Law wrote: > On 09/09/2017 02:51 AM, Eric Botcazou wrote: > >> No, the stack never gets misaligned - my patch doesn't change that at all. > > > > Yes, it does.  Dynamic allocation works like this: the amount to be > > allocated > > is added to VIRTUAL_STACK_DYNAMIC_REGNUM and the result is

Re: [PATCH] Improve alloca alignment

2017-09-11 Thread Jeff Law
On 09/09/2017 02:51 AM, Eric Botcazou wrote: >> No, the stack never gets misaligned - my patch doesn't change that at all. > > Yes, it does. Dynamic allocation works like this: the amount to be allocated > is added to VIRTUAL_STACK_DYNAMIC_REGNUM and the result is then dynamically > aligned.

Re: [PATCH] Improve alloca alignment

2017-09-11 Thread Wilco Dijkstra
Eric Botcazou wrote:   >> No, the stack never gets misaligned - my patch doesn't change that at all. > > Yes, it does.  No. Look at the diffs, there is not a single change in alignment anywhere for all of the alloca variants. If the alignment is incorrect after my patch, it is also incorrect

Re: [PATCH] Improve alloca alignment

2017-09-09 Thread Eric Botcazou
> No, the stack never gets misaligned - my patch doesn't change that at all. Yes, it does. Dynamic allocation works like this: the amount to be allocated is added to VIRTUAL_STACK_DYNAMIC_REGNUM and the result is then dynamically aligned. Your patch assumes that VIRTUAL_STACK_DYNAMIC_REGNUM

Re: [PATCH] Improve alloca alignment

2017-09-08 Thread Wilco Dijkstra
Hi Rainer, Can you post the disassembly for say the 8-byte aligned tests? It may not be built correctly or hit an offset that is accidentally aligned, however pass/fail status can't change due to my patch as it doesn't change alignment at all. Wilco

Re: [PATCH] Improve alloca alignment

2017-09-08 Thread Rainer Orth
Hi Wilco, > Eric Botcazou wrote: > >> The stack is aligned before the allocation but it gets misaligned during the >> allocation because the dynamic offset is not a multiple of STACK_BOUNDARY. > > No, the stack never gets misaligned - my patch doesn't change that at > all. The issue is that Sparc

Re: [PATCH] Improve alloca alignment

2017-09-08 Thread Wilco Dijkstra
Eric Botcazou wrote: > The stack is aligned before the allocation but it gets misaligned during the > allocation because the dynamic offset is not a multiple of STACK_BOUNDARY. No, the stack never gets misaligned - my patch doesn't change that at all. The issue is that Sparc backend doesn't

Re: [PATCH] Improve alloca alignment

2017-09-08 Thread Eric Botcazou
> This patch brought back PR libgomp/78468, which had caused its > predecessor to be backed out of gcc-7. Yes, it's exactly the same mistake: + /* Since the stack is presumed to be aligned before this allocation, + we only need to increase the size of the allocation if the required +

Re: [PATCH] Improve alloca alignment

2017-09-06 Thread Rainer Orth
Jeff Law writes: > On 08/22/2017 08:15 AM, Wilco Dijkstra wrote: >> Jeff Law wrote: >> On 07/26/2017 05:29 PM, Wilco Dijkstra wrote: >> But then the check size_align % MAX_SUPPORTED_STACK_ALIGNMENT != 0 seems wrong too given that round_push uses a different alignment

Re: [PATCH] Improve alloca alignment

2017-08-24 Thread Jeff Law
On 08/22/2017 08:15 AM, Wilco Dijkstra wrote: > Jeff Law wrote: > On 07/26/2017 05:29 PM, Wilco Dijkstra wrote: > >>> But then the check size_align % MAX_SUPPORTED_STACK_ALIGNMENT != 0 >>> seems wrong too given that round_push uses a different alignment to align >>> to. >> I had started to dig

Re: [PATCH] Improve alloca alignment

2017-08-22 Thread Wilco Dijkstra
Jeff Law wrote: On 07/26/2017 05:29 PM, Wilco Dijkstra wrote: > > But then the check size_align % MAX_SUPPORTED_STACK_ALIGNMENT != 0 > > seems wrong too given that round_push uses a different alignment to align > > to. > I had started to dig into the history of this code, but just didn't have >

Re: [PATCH] Improve alloca alignment

2017-07-27 Thread Jeff Law
On 07/26/2017 05:29 PM, Wilco Dijkstra wrote: > Jeff Law wrote: > >> + if (required_align > MAX_SUPPORTED_STACK_ALIGNMENT) >> +{ >> + extra = (required_align - MAX_SUPPORTED_STACK_ALIGNMENT) >> + / BITS_PER_UNIT; >> + size = plus_constant (Pmode, size, extra); >> + size =

Re: [PATCH] Improve alloca alignment

2017-07-26 Thread Wilco Dijkstra
Jeff Law wrote: > +  if (required_align > MAX_SUPPORTED_STACK_ALIGNMENT) > +    { > +  extra = (required_align - MAX_SUPPORTED_STACK_ALIGNMENT) > + / BITS_PER_UNIT; > +  size = plus_constant (Pmode, size, extra); > +  size = force_operand (size, NULL_RTX); >  > -  if (extra &&

Re: [PATCH] Improve alloca alignment

2017-07-26 Thread Jeff Law
On 07/26/2017 11:39 AM, Wilco Dijkstra wrote: > This patch improves alloca alignment. Currently alloca reserves > too much space as it aligns twice, and generates unnecessary stack > alignment code. For example alloca (16) generates: > > sub sp, sp, #32 ??? > mov x1, sp >