[Bug middle-end/46685] [4.6 Regression] New stack alignment failures

[Bug middle-end/46685] [4.6 Regression] New stack alignment failures

[danglin at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

--- Comment #1 from John David Anglin <danglin at gcc dot gnu.org> 2010-11-27 16:33:36 UTC ---
Although the printouts don't show it, the fails are with -fpic.
The non pic versions don't fail.

In the nested-6.c case, it appears the procedure label for the
call to compare from qsort is built incorrectly.  As a result,
the test segfaults in libc.

[Bug middle-end/46685] New: [4.6 Regression] New stack alignment failures

[danglin at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

           Summary: [4.6 Regression] New stack alignment failures
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: danglin@gcc.gnu.org
              Host: hppa-unknown-linux-gnu
            Target: hppa-unknown-linux-gnu
             Build: hppa-unknown-linux-gnu


In r167135, I see the following new fails:

FAIL: gcc.dg/torture/stackalign/nested-6.c  -O1  execution test
FAIL: gcc.dg/torture/stackalign/nested-6.c  -O2  execution test
FAIL: gcc.dg/torture/stackalign/nested-6.c  -O3 -fomit-frame-pointer  execution
test
FAIL: gcc.dg/torture/stackalign/nested-6.c  -O3 -g  execution test
FAIL: gcc.dg/torture/stackalign/nested-6.c  -Os  execution test
FAIL: gcc.dg/torture/stackalign/nested-6.c  -O2 -flto -flto-partition=none 
execution test
FAIL: gcc.dg/torture/stackalign/nested-6.c  -O2 -flto  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-1.c  -O1  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-1.c  -O2  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-1.c  -O3 -fomit-frame-pointer 
execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-1.c  -O3 -fomit-frame-pointer
-funroll-loops  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-1.c  -O3 -fomit-frame-pointer
-funroll-all-loops -finline-functions  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-1.c  -O3 -g  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-1.c  -Os  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-1.c  -O2 -flto
-flto-partition=none  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-1.c  -O2 -flto  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-2.c  -O1  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-2.c  -O2  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-2.c  -O3 -fomit-frame-pointer 
execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-2.c  -O3 -fomit-frame-pointer
-funroll-loops  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-2.c  -O3 -fomit-frame-pointer
-funroll-all-loops -finline-functions  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-2.c  -O3 -g  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-2.c  -Os  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-2.c  -O2 -flto
-flto-partition=none  execution test
FAIL: gcc.dg/torture/stackalign/non-local-goto-2.c  -O2 -flto  execution test

r167084 was ok.

[Bug middle-end/46685] [4.6 Regression] New stack alignment failures

[danglin at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

--- Comment #2 from John David Anglin <danglin at gcc dot gnu.org> 2010-11-27 16:53:41 UTC ---
        ldo -62(%r30),%r23
        bl qsort,%r2

The offset of -62 is misaligned.  r23 should point to a plabel.
plabels have to be 4-byte aligned.

[Bug middle-end/46685] [4.6 Regression] New stack alignment failures

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.6.0

[Bug middle-end/46685] [4.6 Regression] New stack alignment failures

[danglin at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

John David Anglin <danglin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.6.0                       |---

--- Comment #3 from John David Anglin <danglin at gcc dot gnu.org> 2010-11-29 00:52:05 UTC ---
The regression was introduced in r167085.

[Bug middle-end/46685] [4.6 Regression] New stack alignment failures

[danglin at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

John David Anglin <danglin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.6.0

[Bug middle-end/46685] [4.6 Regression] New stack alignment failures

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|hppa-unknown-linux-gnu      |hppa,sparc-unknown-linux-gn
                   |                            |u
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2010.11.29 06:35:10
                 CC|                            |ebotcazou at gcc dot
                   |                            |gnu.org, hubicka at gcc dot
                   |                            |gnu.org
               Host|hppa-unknown-linux-gnu      |hppa,sparc-unknown-linux-gn
                   |                            |u
     Ever Confirmed|0                           |1
              Build|hppa-unknown-linux-gnu      |hppa,sparc-unknown-linux-gn
                   |                            |u

--- Comment #4 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2010-11-29 06:35:10 UTC ---
Same failures on SPARC/Linux.

[Bug middle-end/46685] [4.6 Regression] New stack alignment failures

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2010-11-29 11:46:28 UTC ---
What is the assembly difference on SPARC Linux?  The only difference I see with
a cross compiler to sparc64-linux, both for -O1 -m32 -mptr32 and -O1 -m64 is
.section    .text.startup,"ax",@progbits
line added before main, which I don't see how it should break things up.

[Bug middle-end/46685] [4.6 Regression] New stack alignment failures

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

--- Comment #6 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2010-11-29 11:58:56 UTC ---
> What is the assembly difference on SPARC Linux?  The only difference I see with
> a cross compiler to sparc64-linux, both for -O1 -m32 -mptr32 and -O1 -m64 is
> .section    .text.startup,"ax",@progbits
> line added before main, which I don't see how it should break things up.

The errors are also visible on SPARC/Solaris.  You need -fpic.

FAIL: gcc.dg/torture/stackalign/non-local-goto-1.c  -Os  (test for excess
error\
s)
Excess errors:
/var/tmp//ccC9NwIg.s:27: Error: invalid sections for operation on `.LL3' and
`L\
0^A'
/var/tmp//ccC9NwIg.s:29: Error: invalid sections for operation on `.LL3' and
`L\
0^A'

WARNING: gcc.dg/torture/stackalign/non-local-goto-1.c  -Os  compilation failed
\
to produce executable

[Bug middle-end/46685] [4.6 Regression] New stack alignment failures

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2010-11-29 15:17:30 UTC ---
I guess we could do something like:
--- varasm.c.jj    2010-11-29 12:39:07.000000000 +0100
+++ varasm.c    2010-11-29 15:15:53.000000000 +0100
@@ -534,6 +534,15 @@ section *
 default_function_section (tree decl, enum node_frequency freq,
               bool startup, bool exit)
 {
+  /* Force nested functions into the same section as the containing
+     function.  */
+  if (decl
+      && DECL_SECTION_NAME (decl) == NULL_TREE
+      && DECL_CONTEXT (decl) != NULL_TREE
+      && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL
+      && DECL_SECTION_NAME (DECL_CONTEXT (decl)) == NULL_TREE)
+    return function_section (DECL_CONTEXT (decl));
+
   /* Startup code should go to startup subsection unless it is
      unlikely executed (this happens especially with function splitting
      where we can split away unnecesary parts of static constructors.  */

or perhaps instead of changing default_function_section write a wrapper around
it and use it on targets that need it.  This will use the same section as the
containing function for nested functions.

On the other side, the above is still broken with
-freorder-blocks-and-partition and I guess even before Honza's change it has
been broken.

Alternatively, we could do something like:
--- gcc/config/sparc/sparc.c.jj    2010-11-26 18:39:04.000000000 +0100
+++ gcc/config/sparc/sparc.c    2010-11-29 15:35:00.727219374 +0100
@@ -1066,8 +1066,13 @@ sparc_expand_move (enum machine_mode mod
      are absolutely sure that X is in the same segment as the GOT.
      Unfortunately, the flexibility of linker scripts means that we
      can't be sure of that in general, so assume that _G_O_T_-relative
-     accesses are never valid on VxWorks.  */
-      if (GET_CODE (operands[1]) == LABEL_REF && !TARGET_VXWORKS_RTP)
+     accesses are never valid on VxWorks.
+     If the label is non-local, it might be placed in a different section
+     from . and movMODE_pic_label_ref patterns require the label and .
+     to be in the same section.  */
+      if (GET_CODE (operands[1]) == LABEL_REF
+      && !TARGET_VXWORKS_RTP
+      && !LABEL_REF_NONLOCAL_P (operands[1]))
     {
       if (mode == SImode)
         {

Not sure if even in the current function it could happen that sparc_expand_move
is asked for a label that is in the other partition.  I mean something like:
__attribute__((noinline, noclone))
void bar (void *x)
{
  asm volatile ("" : : "r" (x) : "memory");
}

__attribute__((noinline, noclone))
void baz (void)
{
  asm volatile ("" : : : "memory");
}

__attribute__((noinline, noclone))
int foo (int x)
{
  __label__ lab;
  if (__builtin_expect (x, 0))
    {
      lab:
      baz ();
      return 2;
    }
  bar (&&lab);
  return 1;
}

int
main (void)
{
  int x, i;
  asm volatile ("" : "=r" (x) : "0" (0));
  for (i = 0; i < 1000000; i++)
    foo (x);
  return 0;
}

first compiled/linked with -O2 -fprofile-generate
-freorder-blocks-and-partition -fpic, then executed, then compiled again with
-O2 -fprofile-use -freorder-blocks-and-partition -fpic.  At least on
x86_64-linux the baz () bb is .text.unlikely, while bar (&&lab) is .text
section.  Now, I guess this wouldn't assemble on sparc-linux or Solaris, even
before Honza's patch.

In that case we perhaps could look at LABEL_REF's operand (if any, nonlocal
labels probably don't have them) and look at bb flags of its INSN_BLOCK. 
Unfortunately, the partitions are only computed during *.bbpart, so this is not
known at expansion time (and in sparc_expand_move we might not even know in
which bb we are going to end up).
So, perhaps we'd just need some splitter for these insns and change them to
something else if the current bb and target LABEL_REF have different partition.
Or simply ammend the second patch above and disable this optimization even for
flag_reorder_blocks_and_partition.

[Bug middle-end/46685] [4.6 Regression] New stack alignment failures with -fpic

[dave at hiauly1 dot hia.nrc.ca]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

--- Comment #8 from dave at hiauly1 dot hia.nrc.ca 2010-11-30 21:52:57 UTC ---
> On the other side, the above is still broken with
> -freorder-blocks-and-partition and I guess even before Honza's change it has
> been broken.

flag_reorder_blocks_and_partition is disabled on pa because we don't
support hot and cold partitioning.  Testing you first suggestion.

Dave

[Bug middle-end/46685] [4.6 Regression] New stack alignment failures with -fpic

[dave at hiauly1 dot hia.nrc.ca]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

--- Comment #9 from dave at hiauly1 dot hia.nrc.ca 2010-11-30 22:22:22 UTC ---
On Tue, 30 Nov 2010, John David Anglin wrote:

> > On the other side, the above is still broken with
> > -freorder-blocks-and-partition and I guess even before Honza's change it has
> > been broken.
> 
> flag_reorder_blocks_and_partition is disabled on pa because we don't
> support hot and cold partitioning.  Testing you first suggestion.

This fixes nested-6.c on hppa-unknown-linux-gnu.  Probably fixes
the others.

Dave

[Bug middle-end/46685] [4.6 Regression] New stack alignment failures with -fpic

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> 2010-12-02 08:40:08 UTC ---
I'd actually prefer to go with the sparc.c change (probably including
flag_reorder_blocks_and_partition), then hppa could just write its own
function_section hook as a wrapper around default_function_section.

Eric, what do you think?  I don't have a working SPARC box around right now, so
can't test it.

[Bug middle-end/46685] [4.6 Regression] New stack alignment failures with -fpic

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

--- Comment #11 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2010-12-02 11:30:54 UTC ---
> Alternatively, we could do something like:
> --- gcc/config/sparc/sparc.c.jj    2010-11-26 18:39:04.000000000 +0100
> +++ gcc/config/sparc/sparc.c    2010-11-29 15:35:00.727219374 +0100
> @@ -1066,8 +1066,13 @@ sparc_expand_move (enum machine_mode mod
>       are absolutely sure that X is in the same segment as the GOT.
>       Unfortunately, the flexibility of linker scripts means that we
>       can't be sure of that in general, so assume that _G_O_T_-relative
> -     accesses are never valid on VxWorks.  */
> -      if (GET_CODE (operands[1]) == LABEL_REF && !TARGET_VXWORKS_RTP)
> +     accesses are never valid on VxWorks.
> +     If the label is non-local, it might be placed in a different section
> +     from . and movMODE_pic_label_ref patterns require the label and .
> +     to be in the same section.  */
> +      if (GET_CODE (operands[1]) == LABEL_REF
> +      && !TARGET_VXWORKS_RTP
> +      && !LABEL_REF_NONLOCAL_P (operands[1]))
>      {
>        if (mode == SImode)
>          {
> 
> Not sure if even in current function it could happen that sparc_expand_move
> is asked for a label that is in the other partition.  I mean something like:
> __attribute__((noinline, noclone))
> void bar (void *x)
> {
>   asm volatile ("" : : "r" (x) : "memory");
> }
> 
> __attribute__((noinline, noclone))
> void baz (void)
> {
>   asm volatile ("" : : : "memory");
> }
> 
> __attribute__((noinline, noclone))
> int foo (int x)
> {
>   __label__ lab;
>   if (__builtin_expect (x, 0))
>     {
>       lab:
>       baz ();
>       return 2;
>     }
>   bar (&&lab);
>   return 1;
> }
> 
> int
> main (void)
> {
>   int x, i;
>   asm volatile ("" : "=r" (x) : "0" (0));
>   for (i = 0; i < 1000000; i++)
>     foo (x);
>   return 0;
> }
> 
> first compiled/linked with -O2 -fprofile-generate
> -freorder-blocks-and-partition -fpic, then executed, then compiled again with
> -O2 -fprofile-use -freorder-blocks-and-partition -fpic.  At least on
> x86_64-linux the baz () bb is .text.unlikely, while bar (&&lab) is .text
> section.  Now, I guess this wouldn't assemble on sparc-linux or Solaris, even
> before Honza's patch.

This testcase just crashes the compiler. :-)  But, yes, once the patchlet

Index: config/sparc/sparc.c
===================================================================
--- config/sparc/sparc.c    (revision 167365)
+++ config/sparc/sparc.c    (working copy)
@@ -9453,6 +9485,7 @@ sparc_file_end (void)
       DECL_VISIBILITY (decl) = VISIBILITY_HIDDEN;
       DECL_VISIBILITY_SPECIFIED (decl) = 1;
       allocate_struct_function (decl, true);
+      cfun->is_thunk = 1;
       current_function_decl = decl;
       init_varasm_status ();
       assemble_start_function (decl, name);

is applied, your prediction is verified:

ebotcazou@grobluk:~/obj$ gcc/xgcc -Bgcc -O2 -fprofile-use
-freorder-blocks-and-partition -fpic pr46685.c
/tmp/ccg98MwA.s: Assembler messages:
/tmp/ccg98MwA.s:71: Error: operation combines symbols in different segments
/tmp/ccg98MwA.s:72: Error: operation combines symbols in different segments

> Or simply amend the second patch above and disable this optimization even for
> flag_reorder_blocks_and_partition.

This is good enough I'd think.  Testing...

[Bug target/46685] [4.6 Regression] New stack alignment failures with -fpic

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

--- Comment #12 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2010-12-02 22:33:24 UTC ---
Author: ebotcazou
Date: Thu Dec  2 22:33:16 2010
New Revision: 167395

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=167395
Log:
    PR target/46685
    * config/sparc/sparc.c (can_use_mov_pic_label_ref): New predicate.
    (sparc_expand_move): Call it to decide whether to emit the special
    mov{si,di}_pic_label_ref patterns.
    (sparc_legitimize_pic_address): Call it to decide whether to emit
    the regular PIC sequence for labels.  Fix long line.
    (sparc_file_end): Set is_thunk for the PIC helper.

Added:
    trunk/gcc/testsuite/gcc.dg/pr46685.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sparc/sparc.c
    trunk/gcc/testsuite/ChangeLog

[Bug target/46685] [4.6 Regression] New stack alignment failures with -fpic

[danglin at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

--- Comment #13 from John David Anglin <danglin at gcc dot gnu.org> 2010-12-08 01:26:48 UTC ---
Author: danglin
Date: Wed Dec  8 01:26:44 2010
New Revision: 167581

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=167581
Log:
    PR middle-end/46671
    PR target/46685
    * config/pa/pa.c (pa_function_section): New function.
    (TARGET_ASM_FUNCTION_SECTION): Define.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/pa/pa.c

[Bug target/46685] [4.6 Regression] New stack alignment failures with -fpic

[danglin at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

John David Anglin <danglin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #14 from John David Anglin <danglin at gcc dot gnu.org> 2010-12-08 01:55:53 UTC ---
Fixed.

[Bug target/46685] [4.6 Regression] New stack alignment failures with -fpic

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46685

--- Comment #15 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-01-03 12:06:23 UTC ---
Author: ebotcazou
Date: Mon Jan  3 12:06:19 2011
New Revision: 168418

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168418
Log:
    Backport from mainline
    2010-12-30  Eric Botcazou  <ebotcazou@adacore.com>

    PR target/47038
    * config/sparc/sparc.c (sparc_file_end): Call resolve_unique_section
    on the GOT helper if USE_HIDDEN_LINKONCE.

    2010-12-02  Eric Botcazou  <ebotcazou@adacore.com>

    PR target/46685
    * config/sparc/sparc.c (can_use_mov_pic_label_ref): New predicate.
    (sparc_expand_move): Call it to decide whether to emit the special
    mov{si,di}_pic_label_ref patterns.
    (sparc_legitimize_pic_address): Call it to decide whether to emit
    the regular PIC sequence for labels.  Fix long line.
    (sparc_file_end): Set is_thunk for the PIC helper.

Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/config/sparc/sparc.c