| Message ID | 1944532.Kj3BqOocuy@polaris |
|---|---|
| State | New |
| Headers | show |
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
> 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.
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
> 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.
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
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)); }
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.