* [rs6000] Fix ICE with -fstack-limit-register and large frames
@ 2017-05-24 7:53 Eric Botcazou
2017-05-26 13:25 ` Segher Boessenkool
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2017-05-24 7:53 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]
Hi,
this fixes an internal error with -fstack-limit-register and large frames:
eric@polaris:~/build/gcc/powerpc-linux> gcc/xgcc -Bgcc -S stack-limit-1.c -
fstack-limit-register=r2
stack-limit-1.c: In function 'foo':
stack-limit-1.c:9:1: error: insn does not satisfy its constraints:
}
^
(insn 21 20 22 (set (reg:SI 0 0)
(plus:SI (reg:SI 0 0)
(const_int 3968 [0xf80]))) "stack-limit-1.c":5 70 {*addsi3}
(nil))
stack-limit-1.c:9:1: internal compiler error: in final_scan_insn, at
final.c:2964
0xb40335 _fatal_insn(char const*, rtx_def const*, char const*, int, char
const*)
/home/eric/svn/gcc/gcc/rtl-error.c:108
0xb4037a _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
/home/eric/svn/gcc/gcc/rtl-error.c:119
0x83f757 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
/home/eric/svn/gcc/gcc/final.c:2964
0x83feb2 final(rtx_insn*, _IO_FILE*, int)
/home/eric/svn/gcc/gcc/final.c:2051
0x84076d rest_of_handle_final
/home/eric/svn/gcc/gcc/final.c:4489
0x84076d execute
/home/eric/svn/gcc/gcc/final.c:4562
Please submit a full bug report,
Tested on PowerPC/Linux, OK for mainline?
2017-05-24 Eric Botcazou <ebotcazou@adacore.com>
* config/rs6000/rs6000.c (rs6000_emit_allocate_stack): Deal properly
with large frames if a stack limit is used.
2017-05-24 Eric Botcazou <ebotcazou@adacore.com>
* gcc.target/powerpc/stack-limit-1.c: New test.
--
Eric Botcazou
[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 893 bytes --]
Index: config/rs6000/rs6000.c
===================================================================
--- config/rs6000/rs6000.c (revision 248140)
+++ config/rs6000/rs6000.c (working copy)
@@ -28372,7 +28372,21 @@ rs6000_emit_allocate_stack (HOST_WIDE_IN
&& REGNO (stack_limit_rtx) > 1
&& REGNO (stack_limit_rtx) <= 31)
{
- emit_insn (gen_add3_insn (tmp_reg, stack_limit_rtx, GEN_INT (size)));
+ rtx cst = GEN_INT (size);
+
+ /* The add expander doesn't correctly handle r0. */
+ if (satisfies_constraint_I (cst))
+ emit_insn (gen_rtx_SET (tmp_reg,
+ gen_rtx_PLUS (Pmode, stack_limit_rtx,
+ cst)));
+ else
+ {
+ emit_move_insn (tmp_reg, cst);
+ emit_insn (gen_rtx_SET (tmp_reg,
+ gen_rtx_PLUS (Pmode, stack_limit_rtx,
+ tmp_reg)));
+ }
+
emit_insn (gen_cond_trap (LTU, stack_reg, tmp_reg,
const0_rtx));
}
[-- Attachment #3: stack-limit-1.c --]
[-- Type: text/x-csrc, Size: 129 bytes --]
/* { dg-do compile } */
/* { dg-options "-fstack-limit-register=r2" } */
int foo (int i)
{
char arr[135000];
arr[i] = 0;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [rs6000] Fix ICE with -fstack-limit-register and large frames
2017-05-24 7:53 [rs6000] Fix ICE with -fstack-limit-register and large frames Eric Botcazou
@ 2017-05-26 13:25 ` Segher Boessenkool
2017-06-02 8:27 ` Eric Botcazou
0 siblings, 1 reply; 6+ messages in thread
From: Segher Boessenkool @ 2017-05-26 13:25 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
Hi!
[ please cc: me and David on rs6000 patches ]
On Wed, May 24, 2017 at 09:51:46AM +0200, Eric Botcazou wrote:
> this fixes an internal error with -fstack-limit-register and large frames:
>
> eric@polaris:~/build/gcc/powerpc-linux> gcc/xgcc -Bgcc -S stack-limit-1.c -
> fstack-limit-register=r2
> stack-limit-1.c: In function 'foo':
> stack-limit-1.c:9:1: error: insn does not satisfy its constraints:
> }
> ^
> (insn 21 20 22 (set (reg:SI 0 0)
> (plus:SI (reg:SI 0 0)
> (const_int 3968 [0xf80]))) "stack-limit-1.c":5 70 {*addsi3}
> (nil))
Yeah, that instruction does not exist.
> Index: config/rs6000/rs6000.c
> ===================================================================
> --- config/rs6000/rs6000.c (revision 248140)
> +++ config/rs6000/rs6000.c (working copy)
> @@ -28372,7 +28372,21 @@ rs6000_emit_allocate_stack (HOST_WIDE_IN
> && REGNO (stack_limit_rtx) > 1
> && REGNO (stack_limit_rtx) <= 31)
> {
> - emit_insn (gen_add3_insn (tmp_reg, stack_limit_rtx, GEN_INT (size)));
> + rtx cst = GEN_INT (size);
> +
> + /* The add expander doesn't correctly handle r0. */
Could you make the expander handle it, instead? It's as simple as (after
the double-reg thing) add "if operands[1] is reg 0, force_reg operands[2]".
I'll do it if you prefer.
[ the patch is broken here ]
> /* { dg-do compile } */
> /* { dg-options "-fstack-limit-register=r2" } */
Please use a different register, r2 already has different functions in
most ABIs. It *probably* will compile anyway, but :-)
Segher
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [rs6000] Fix ICE with -fstack-limit-register and large frames
2017-05-26 13:25 ` Segher Boessenkool
@ 2017-06-02 8:27 ` Eric Botcazou
2017-06-02 16:25 ` Segher Boessenkool
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2017-06-02 8:27 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches
> Could you make the expander handle it, instead? It's as simple as (after
> the double-reg thing) add "if operands[1] is reg 0, force_reg operands[2]".
> I'll do it if you prefer.
Probably, because I'm not sure how this can work, as you cannot create new
pseudos here.
> [ the patch is broken here ]
It applies just fine for me though. But it could probably use add_operand
instead of satisfies_constraint_I in the condition.
> > /* { dg-do compile } */
> > /* { dg-options "-fstack-limit-register=r2" } */
>
> Please use a different register, r2 already has different functions in
> most ABIs. It *probably* will compile anyway, but :-)
It's a straight copy of gcc.target/powerpc/pr48344-1.c though.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [rs6000] Fix ICE with -fstack-limit-register and large frames
2017-06-02 8:27 ` Eric Botcazou
@ 2017-06-02 16:25 ` Segher Boessenkool
2017-06-03 10:34 ` Eric Botcazou
0 siblings, 1 reply; 6+ messages in thread
From: Segher Boessenkool @ 2017-06-02 16:25 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On Fri, Jun 02, 2017 at 10:27:33AM +0200, Eric Botcazou wrote:
> > Could you make the expander handle it, instead? It's as simple as (after
> > the double-reg thing) add "if operands[1] is reg 0, force_reg operands[2]".
> > I'll do it if you prefer.
>
> Probably, because I'm not sure how this can work, as you cannot create new
> pseudos here.
Because you cannot during reload, or another reason? We always use LRA
on powerpc nowadays, and LRA can deal with this.
> > [ the patch is broken here ]
>
> It applies just fine for me though. But it could probably use add_operand
> instead of satisfies_constraint_I in the condition.
Only the first hunk (rs6000.md) applies, the rest is ignored (there is a
blank line here instead of a diff header).
add_operand would be better, yeah.
> > > /* { dg-do compile } */
> > > /* { dg-options "-fstack-limit-register=r2" } */
> >
> > Please use a different register, r2 already has different functions in
> > most ABIs. It *probably* will compile anyway, but :-)
>
> It's a straight copy of gcc.target/powerpc/pr48344-1.c though.
I see. We'll fix it :-)
Segher
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [rs6000] Fix ICE with -fstack-limit-register and large frames
2017-06-02 16:25 ` Segher Boessenkool
@ 2017-06-03 10:34 ` Eric Botcazou
2017-06-03 19:56 ` Segher Boessenkool
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2017-06-03 10:34 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches
> Because you cannot during reload, or another reason? We always use LRA
> on powerpc nowadays, and LRA can deal with this.
Because you cannot during prologue/epilogue generation.
> Only the first hunk (rs6000.md) applies, the rest is ignored (there is a
> blank line here instead of a diff header).
!?? The patch contains a single hunk for config/rs6000/rs6000.c.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [rs6000] Fix ICE with -fstack-limit-register and large frames
2017-06-03 10:34 ` Eric Botcazou
@ 2017-06-03 19:56 ` Segher Boessenkool
0 siblings, 0 replies; 6+ messages in thread
From: Segher Boessenkool @ 2017-06-03 19:56 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On Sat, Jun 03, 2017 at 12:34:21PM +0200, Eric Botcazou wrote:
> > Because you cannot during reload, or another reason? We always use LRA
> > on powerpc nowadays, and LRA can deal with this.
>
> Because you cannot during prologue/epilogue generation.
Ah, this code is generated only then, I see now.
> > Only the first hunk (rs6000.md) applies, the rest is ignored (there is a
> > blank line here instead of a diff header).
>
> !?? The patch contains a single hunk for config/rs6000/rs6000.c.
The second hunk is the testcase. I now see it isn't even part of the
patch, just pasted on.
I opened PR80966. Thanks,
Segher
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-06-03 19:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-24 7:53 [rs6000] Fix ICE with -fstack-limit-register and large frames Eric Botcazou
2017-05-26 13:25 ` Segher Boessenkool
2017-06-02 8:27 ` Eric Botcazou
2017-06-02 16:25 ` Segher Boessenkool
2017-06-03 10:34 ` Eric Botcazou
2017-06-03 19:56 ` Segher Boessenkool
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).