[Bug target/50091] New: [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[Bug target/50091] New: [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[dominiq at lps dot ens.fr]

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

             Bug #: 50091
           Summary: [4.5/4.6/4.7 Regression] -fstack-check gives bad
                    assembly on powerpc-apple-darwin9
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: dominiq@lps.ens.fr
                CC: iains@gcc.gnu.org, spop@gcc.gnu.org
              Host: powerpc-apple-darwin9
            Target: powerpc-apple-darwin9
             Build: powerpc-apple-darwin9


On powerpc-apple-darwin9 I get

FAIL: gcc.dg/graphite/run-id-pr47653.c (test for excess errors)

The error is

[karma] f90/bug% gcc47 -O -fstack-check=generic -ftree-pre -fgraphite-identity
/opt/gcc/work/gcc/testsuite/gcc.dg/graphite/run-id-pr47653.c
/var/tmp//ccQC61mg.s:12:Parameter syntax error (parameter 1)
/var/tmp//ccQC61mg.s:13:Parameter syntax error (parameter 1)

The syntax error comming from "stw 0,..." in

...
        stw 0,-12284(r1)
        mr r0,r1
        stw 0,-12556(r1)
...

It turns out that -fstack-check=generic (or -fstack-check) is enough to trigger
the error for gcc 4.5.3, 4.6.1, and trunk, but not for 4.4.6. I have tested
some C files and this seems quite generic although
gcc/testsuite/gcc.dg/pr48134.c assembles with -O or -O1, but not with -O2.

QUESTION for graphite: is -fstack-check=generic really necessary for
gcc.dg/graphite/run-id-pr47653.c?

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot
                   |                            |gnu.org
   Target Milestone|---                         |4.5.4

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-16 08:29:39 UTC ---
Well, look at the PR - it was an ICE with graphite and stack-check, so yes,
of course.

        stw 0,-12284(r1)

looks like some missing operand print thing to dump fancy regnames (r0 instead
of 0).

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[ebotcazou at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2011-08-16
         AssignedTo|unassigned at gcc dot       |ebotcazou at gcc dot
                   |gnu.org                     |gnu.org
     Ever Confirmed|0                           |1

--- Comment #2 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-08-16 17:12:00 UTC ---
> Well, look at the PR - it was an ICE with graphite and stack-check, so yes,
> of course.
> 
>         stw 0,-12284(r1)
> 
> looks like some missing operand print thing to dump fancy regnames (r0 instead
> of 0).

How come the assembler chokes on this?  Looking into it...

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[pinskia at gmail dot com]

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

--- Comment #3 from pinskia at gmail dot com <pinskia at gmail dot com> 2011-08-16 17:25:26 UTC ---
Because darwin's as does not support it. It only supports with r0.



Sent from my Palm Pre on AT&amp;T
On Aug 16, 2011 10:13, ebotcazou at gcc dot gnu.org
&lt;gcc-bugzilla@gcc.gnu.org&gt; wrote: 

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



Eric Botcazou &lt;ebotcazou at gcc dot gnu.org&gt; changed:



           What    |Removed                     |Added

----------------------------------------------------------------------------

             Status|UNCONFIRMED                 |ASSIGNED

   Last reconfirmed|                            |2011-08-16

         AssignedTo|unassigned at gcc dot       |ebotcazou at gcc dot

                   |gnu.org                     |gnu.org

     Ever Confirmed|0                           |1



--- Comment #2 from Eric Botcazou &lt;ebotcazou at gcc dot gnu.org&gt;
2011-08-16 17:12:00 UTC ---

&gt; Well, look at the PR - it was an ICE with graphite and stack-check, so
yes,

&gt; of course.

&gt; 

&gt;         stw 0,-12284(r1)

&gt; 

&gt; looks like some missing operand print thing to dump fancy regnames (r0
instead

&gt; of 0).



How come the assembler chokes on this?  Looking into it...

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[dominiq at lps dot ens.fr]

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

--- Comment #4 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2011-08-17 22:32:23 UTC ---
If I edit the assembly code to have

...
        stw r0,-12284(r1)
        mr r0,r1
        stw r0,-12556(r1)
...

The code assembles, links and runs without further hiccup.

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[iains at gcc dot gnu.org]

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

--- Comment #5 from Iain Sandoe <iains at gcc dot gnu.org> 2011-08-20 13:02:19 UTC ---
(In reply to comment #4)
> If I edit the assembly code to have
> 
> ...
>         stw r0,-12284(r1)
>         mr r0,r1
>         stw r0,-12556(r1)
> ...
> 
> The code assembles, links and runs without further hiccup.

rs6000.md:

(define_insn "probe_stack"
  [(set (match_operand 0 "memory_operand" "=m")
        (unspec [(const_int 0)] UNSPEC_PROBE_STACK))]
  ""
  "{st%U0%X0|stw%U0%X0} 0,%0"
  [(set_attr "type" "store")
   (set_attr "length" "4")])

it appears (to one with near zero experience of md files) that the intent of
the insn is to store a  value (0) into the location (thus probing the stack). 
Presumably, one gets a segfault or bus error in the case of fail.

The i386 equivalent stores $0 into the location ... but I suppose any value or
register would do.

Presumably, for PPC assemblers that don't use the 'r' prefix on the registers,
it will be storing r0 into the slot (effectively, what Dominique's hand-edit is
doing). 

 I guess there's some really obvious way to fix this... but I don't (yet) know
the syntax of the md stuff ..

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[schwab@linux-m68k.org]

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

--- Comment #6 from Andreas Schwab <schwab@linux-m68k.org> 2011-08-20 14:49:14 UTC ---
"st 0,%0" stores the contents of register 0 at the address pointed to by
operand 0.  The st insn always operates on registers and cannot take immediate
data.

There is no modifier that expands to the register prefix.  Currently the only
way to produce the correct register name is to substitute a suitable register
operand.

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[ebotcazou at gcc dot gnu.org]

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

--- Comment #7 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-09-03 13:28:43 UTC ---
Created attachment 25182
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25182
Tentative fix

Untested as of this writing.

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[iains at gcc dot gnu.org]

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

--- Comment #8 from Iain Sandoe <iains at gcc dot gnu.org> 2011-09-03 14:17:12 UTC ---
(In reply to comment #7)
> Created attachment 25182 [details]
> Tentative fix
> 
> Untested as of this writing.

Thanks, starting bootstrap in a minute .. .

... your patch +  this (and some unrelated fixes for powerpc ADA bootstrap):

Index: gcc/config/rs6000/rs6000.c
===================================================================
@@ -19303,7 +19308,10 @@
   output_asm_insn ("{cal %0,%1(%0)|addi %0,%0,%1}", xops);

   /* Probe at TEST_ADDR and branch.  */
-  output_asm_insn ("{st|stw} 0,0(%0)", xops);
+  if (TARGET_MACHO)
+    output_asm_insn ("{st|stw} r0,0(%0)", xops);
+  else
+    output_asm_insn ("{st|stw} 0,0(%0)", xops);
   fprintf (asm_out_file, "\tb ");
   assemble_name_raw (asm_out_file, loop_lab);
   fputc ('\n', asm_out_file);

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[ebotcazou at gcc dot gnu.org]

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

--- Comment #9 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-09-03 14:45:01 UTC ---
> Thanks, starting bootstrap in a minute .. .
> 
> ... your patch +  this (and some unrelated fixes for powerpc ADA bootstrap):
> 
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> @@ -19303,7 +19308,10 @@
>    output_asm_insn ("{cal %0,%1(%0)|addi %0,%0,%1}", xops);
> 
>    /* Probe at TEST_ADDR and branch.  */
> -  output_asm_insn ("{st|stw} 0,0(%0)", xops);
> +  if (TARGET_MACHO)
> +    output_asm_insn ("{st|stw} r0,0(%0)", xops);
> +  else
> +    output_asm_insn ("{st|stw} 0,0(%0)", xops);
>    fprintf (asm_out_file, "\tb ");
>    assemble_name_raw (asm_out_file, loop_lab);
>    fputc ('\n', asm_out_file);

Ah, thanks, I missed this.  Something like the to-be-attached patch would be
more correct though, I think.

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[ebotcazou at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #25182|0                           |1
        is obsolete|                            |

--- Comment #10 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-09-03 14:46:20 UTC ---
Created attachment 25183
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25183
Tentative fix (2)

Still untested.

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[iains at gcc dot gnu.org]

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

--- Comment #11 from Iain Sandoe <iains at gcc dot gnu.org> 2011-09-03 16:48:18 UTC ---
(In reply to comment #10)
> Created attachment 25183 [details]
> Tentative fix (2)
> 
> Still untested.

bootstrapped with your amended change to rs6000.c
./gcc/xgcc -Bgcc ../tests/hello.c -o hc -fstack-check -save-temps -fverbose-asm
-fdump-rtl-all
... shows that the stack check is present and correct ... 
any idea what test would produce the stack range check?

(make check is running - but will take some time).

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[ebotcazou at gcc dot gnu.org]

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

--- Comment #12 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-09-03 17:28:32 UTC ---

> bootstrapped with your amended change to rs6000.c
> ./gcc/xgcc -Bgcc ../tests/hello.c -o hc -fstack-check -save-temps -fverbose-asm
> -fdump-rtl-all
> ... shows that the stack check is present and correct ... 
> any idea what test would produce the stack range check?

You'd need to define STACK_CHECK_STATIC_BUILTIN to 1 for PowerPC/Darwin and
have a large frame.

> (make check is running - but will take some time).

Thanks for the testing.

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[iains at gcc dot gnu.org]

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

--- Comment #13 from Iain Sandoe <iains at gcc dot gnu.org> 2011-09-04 11:18:23 UTC ---
(In reply to comment #12)
> > bootstrapped with your amended change to rs6000.c
> > ./gcc/xgcc -Bgcc ../tests/hello.c -o hc -fstack-check -save-temps -fverbose-asm
> > -fdump-rtl-all
> > ... shows that the stack check is present and correct ... 
> > any idea what test would produce the stack range check?
> 
> You'd need to define STACK_CHECK_STATIC_BUILTIN to 1 for PowerPC/Darwin and
> have a large frame.

re-bootstrapped (less Ada and Java) with that ...

... checked manually with a cooked-up testcase that we get:
(insn 683 682 684 (set (reg:SI 12 r12)
        (unspec_volatile:SI [
                (reg:SI 12 r12)
                (reg:SI 0 r0)
            ] UNSPECV_PROBE_STACK_RANGE)) ../test-ppc/limit-struct.c:21 441
{probe_stack_rangesi}
     (nil))

for a large frame ... and:

(insn 30 4 31 (set (mem/v:SI (plus:SI (reg/f:SI 1 r1)
                (const_int -12368 [0xffffffffffffcfb0])) [0 S4 A8])
        (unspec [
                (const_int 0 [0])
            ] UNSPEC_PROBE_STACK)) ../test-ppc/limit-struct.c:93 440
{probe_stack}
     (nil))

for a smaller frame ...
... and the asm looks sensible...

... running check again ... but I'd hazard that this is fairly safe (at least,
from the darwin perspective).   However, I'm not able to check ppc linux here, 
as things stand.

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[ebotcazou at gcc dot gnu.org]

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

--- Comment #14 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-09-04 13:02:15 UTC ---
> for a smaller frame ... and the asm looks sensible...

Great, thanks.

Defining STACK_CHECK_STATIC_BUILTIN to 1 for Darwin would be a separate thing.
In particular, you'd need to test Ada to validate this change.

> ... running check again ... but I'd hazard that this is fairly safe (at least,
> from the darwin perspective).   However, I'm not able to check ppc linux here, 
> as things stand.

I'll give it a whirl on PowerPC/Linux.

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check gives bad assembly on powerpc-apple-darwin9

[iains at gcc dot gnu.org]

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

--- Comment #15 from Iain Sandoe <iains at gcc dot gnu.org> 2011-09-04 15:28:45 UTC ---
(In reply to comment #14)

> Defining STACK_CHECK_STATIC_BUILTIN to 1 for Darwin would be a separate thing.
> In particular, you'd need to test Ada to validate this change.

indeed, lucky I finally figured out how to get Ada to bootstrap on
powerpc-darwin9 then ;-)

(FTR, STACK_CHECK_STATIC_BUILTIN is OK on i686-darwin9 and x86-64-darwin10 is
running)

> > ... running check again ... 

... the tests were OK with the c-family + fortran (STACK_CHECK_STATIC_BUILTIN +
comment 12).

I'm about to reg-strap w/Ada & java to check various WIP.. (long job, don't
expect an answer today) ..

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check generates wrong assembly

[ebotcazou at gcc dot gnu.org]

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

--- Comment #16 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-09-18 22:00:57 UTC ---
Author: ebotcazou
Date: Sun Sep 18 22:00:52 2011
New Revision: 178944

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=178944
Log:
    PR target/50091
    * config/rs6000/rs6000.md (probe_stack): Use explicit operand.
    * config/rs6000/rs6000.c (output_probe_stack_range): Likewise.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000.c
    trunk/gcc/config/rs6000/rs6000.md

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check generates wrong assembly

[ebotcazou at gcc dot gnu.org]

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

--- Comment #17 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-09-18 22:02:01 UTC ---
Author: ebotcazou
Date: Sun Sep 18 22:01:56 2011
New Revision: 178945

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=178945
Log:
    PR target/50091
    * config/rs6000/rs6000.md (probe_stack): Use explicit operand.
    * config/rs6000/rs6000.c (output_probe_stack_range): Likewise.

Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/config/rs6000/rs6000.c
    branches/gcc-4_6-branch/gcc/config/rs6000/rs6000.md

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check generates wrong assembly

[ebotcazou at gcc dot gnu.org]

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

--- Comment #18 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-09-18 22:02:31 UTC ---
Author: ebotcazou
Date: Sun Sep 18 22:02:27 2011
New Revision: 178946

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=178946
Log:
    PR target/50091
    * config/rs6000/rs6000.md (probe_stack): Use explicit operand.

Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/config/rs6000/rs6000.md

[Bug target/50091] [4.5/4.6/4.7 Regression] -fstack-check generates wrong assembly

[ebotcazou at gcc dot gnu.org]

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

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

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

--- Comment #19 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-09-18 22:04:44 UTC ---
At last.