[Bug middle-end/78468] [8 regression] libgomp.c/reduction-10.c and many more FAIL

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78468

Eric Botcazou  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #46 from Eric Botcazou  ---
.

[Bug middle-end/78468] [8 regression] libgomp.c/reduction-10.c and many more FAIL

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78468

--- Comment #45 from Eric Botcazou  ---
Author: ebotcazou
Date: Wed Dec 13 23:16:56 2017
New Revision: 255616

URL: https://gcc.gnu.org/viewcvs?rev=255616=gcc=rev
Log:
PR middle-end/78468
* emit-rtl.c (init_emit): Remove ??? comment.
* explow.c (get_dynamic_stack_size): Take known alignment of stack
pointer + STACK_DYNAMIC_OFFSET into account in lieu of STACK_BOUNDARY.
* config/sparc/sparc.h (INIT_EXPANDERS): In 32-bit mode, lower the
alignment of 3 virtual registers to BITS_PER_WORD.

* config/sparc/sparc.c (sparc_compute_frame_size): Simplify.

Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/sparc/sparc.c
trunk/gcc/config/sparc/sparc.h
trunk/gcc/emit-rtl.c
trunk/gcc/explow.c

[Bug middle-end/78468] [8 regression] libgomp.c/reduction-10.c and many more FAIL

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78468

Eric Botcazou  changed:

   What|Removed |Added

 Status|REOPENED|ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |ebotcazou at gcc dot 
gnu.org

--- Comment #44 from Eric Botcazou  ---
Fixing again.

[Bug middle-end/78468] [8 regression] libgomp.c/reduction-10.c and many more FAIL

[wilco at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78468

--- Comment #43 from Wilco  ---
Author: wilco
Date: Wed Sep  6 16:34:54 2017
New Revision: 251811

URL: https://gcc.gnu.org/viewcvs?rev=251811=gcc=rev
Log:
PR78468 - add alloca alignment test

Add an alignment test to check that aligned alloca's really do get
correctly aligned.  Some targets may not ensure SP is always a multiple
of STACK_BOUNDARY (particularly with outgoing arguments), which means
aligned alloca does not get correctly aligned.  This can be fixed either
by aligning the outgoing arguments or setting STACK_BOUNDARY correctly.

testsuite/
PR middle-end/78468
* gcc.dg/pr78468.c: Add alignment test.

Added:
trunk/gcc/testsuite/gcc.dg/pr78468.c
Modified:
trunk/gcc/testsuite/ChangeLog

[Bug middle-end/78468] [8 regression] libgomp.c/reduction-10.c and many more FAIL

[wilco at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78468

--- Comment #42 from Wilco  ---
(In reply to Eric Botcazou from comment #41)
> > If you cannot guarantee the alignment of the pointers to STACK_BOUNDARY then
> > STACK_BOUNDARY is incorrect.
> 
> No, it is correct as per the definition:
> 
>  -- Macro: STACK_BOUNDARY
>  Define this macro to the minimum alignment enforced by hardware
>  for the stack pointer on this machine.  The definition is a C
>  expression for the desired alignment (measured in bits).  This
>  value is used as a default if `PREFERRED_STACK_BOUNDARY' is not
>  defined.  On most machines, this should be the same as
>  `PARM_BOUNDARY'.

Indeed, so that means alloca code can safely rely on SP being aligned to
STACK_BOUNDARY.

> > GCC uses the STACK_BOUNDARY guarantee in optimizations so it is essential 
> > to 
> > get this right if you want correct code  generation.
> 
> No, you're interpolating, please read the entire discussion.  Your change is
> based on a premise that is wrong at least on 32-bit SPARC.

Yes I did, and it is quite clear - if SPARC doesn't guarantee alignment of the
pointers, it setting of STACK_BOUNDARY is simply incorrect. Alloca will access
data below SP if SP is not aligned to STACK_BOUNDARY even before my patch. No
amount of extra padding to try hiding the bug will fix that.

[Bug middle-end/78468] [8 regression] libgomp.c/reduction-10.c and many more FAIL

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78468

--- Comment #41 from Eric Botcazou  ---
> If you cannot guarantee the alignment of the pointers to STACK_BOUNDARY then
> STACK_BOUNDARY is incorrect.

No, it is correct as per the definition:

 -- Macro: STACK_BOUNDARY
 Define this macro to the minimum alignment enforced by hardware
 for the stack pointer on this machine.  The definition is a C
 expression for the desired alignment (measured in bits).  This
 value is used as a default if `PREFERRED_STACK_BOUNDARY' is not
 defined.  On most machines, this should be the same as
 `PARM_BOUNDARY'.

> GCC uses the STACK_BOUNDARY guarantee in optimizations so it is essential to 
> get this right if you want correct code  generation.

No, you're interpolating, please read the entire discussion.  Your change is
based on a premise that is wrong at least on 32-bit SPARC.

[Bug middle-end/78468] [8 regression] libgomp.c/reduction-10.c and many more FAIL

[wdijkstr at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78468

--- Comment #40 from Wilco  ---
(In reply to Eric Botcazou from comment #39)
> > The existing alloca code relies on STACK_BOUNDARY being set correctly. Has
> > the value been fixed already for the OS variants mentioned? If stack
> > alignment can't be guaranteed by OS/runtime/prolog, STACK_BOUNDARY should be
> > 8.
> 
> No, no, no, please read the ??? note I put in emit-rtl.c.  STACK_BOUNDARY is
> correct, the problem is the declared alignment of the virtual registers.

If you cannot guarantee the alignment of the pointers to STACK_BOUNDARY then
STACK_BOUNDARY is incorrect. GCC uses the STACK_BOUNDARY guarantee in
optimizations so it is essential to get this right if you want correct code
generation.

[Bug middle-end/78468] [8 regression] libgomp.c/reduction-10.c and many more FAIL

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78468

--- Comment #39 from Eric Botcazou  ---
> The existing alloca code relies on STACK_BOUNDARY being set correctly. Has
> the value been fixed already for the OS variants mentioned? If stack
> alignment can't be guaranteed by OS/runtime/prolog, STACK_BOUNDARY should be
> 8.

No, no, no, please read the ??? note I put in emit-rtl.c.  STACK_BOUNDARY is
correct, the problem is the declared alignment of the virtual registers.

[Bug middle-end/78468] [8 regression] libgomp.c/reduction-10.c and many more FAIL

[wdijkstr at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78468

Wilco  changed:

   What|Removed |Added

 CC||wdijkstr at arm dot com

--- Comment #38 from Wilco  ---
(In reply to Rainer Orth from comment #37)
> This patch
> 
> 2017-09-05  Wilco Dijkstra  
> 
>   * explow.c (get_dynamic_stack_size): Improve dynamic alignment.
> 
> brought the exact same set of failures back on sparc-sun-solaris2.11.
> 
>   Rainer

The existing alloca code relies on STACK_BOUNDARY being set correctly. Has the
value been fixed already for the OS variants mentioned? If stack alignment
can't be guaranteed by OS/runtime/prolog, STACK_BOUNDARY should be 8.

[Bug middle-end/78468] [8 regression] libgomp.c/reduction-10.c and many more FAIL

[ro at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78468

Rainer Orth  changed:

   What|Removed |Added

 Status|RESOLVED|REOPENED
   Last reconfirmed|2016-11-25 00:00:00 |2017-9-6
 CC||wilco at gcc dot gnu.org
Version|7.0 |8.0
 Resolution|FIXED   |---
   Target Milestone|7.0 |8.0
Summary|[7 regression]  |[8 regression]
   |libgomp.c/reduction-10.c|libgomp.c/reduction-10.c
   |and many more FAIL  |and many more FAIL

--- Comment #37 from Rainer Orth  ---
This patch

2017-09-05  Wilco Dijkstra  

* explow.c (get_dynamic_stack_size): Improve dynamic alignment.

brought the exact same set of failures back on sparc-sun-solaris2.11.

  Rainer