| Message ID | 201403271849.s2RIn7hS028947@d03av02.boulder.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Mar 27, 2014 at 2:49 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote: > Hello, > > when trying to build Ada for powerpc64le-linux, I ran into an ICE > in fixup_args_size_notes. > > It turns out that the p8 fusion peephole acts on these two insns > from the epilog sequence: > > (insn 1693 1078 1079 91 (set (reg:DI 7 7) > (plus:DI (reg/f:DI 31 31) > (const_int 65536 [0x10000]))) 82 {*adddi3_internal1} > (nil)) > (insn 1079 1693 1511 91 (set (reg/f:DI 1 1) > (mem/c:DI (plus:DI (reg:DI 7 7) > (const_int -16096 [0xffffffffffffc120])) [233 %sfp+49440 S8 A64])) 519 {*movdi_internal64} > (expr_list:REG_DEAD (reg:DI 7 7) > (expr_list:REG_ARGS_SIZE (const_int 0 [0]) > (nil)))) > > and replaces them by: > > (insn 1776 1078 1777 91 (set (reg/f:DI 1 1) > (plus:DI (reg/f:DI 31 31) > (const_int 65536 [0x10000]))) -1 > (nil)) > > (insn 1777 1776 1511 91 (set (reg/f:DI 1 1) > (mem/c:DI (plus:DI (reg/f:DI 1 1) > (const_int -16096 [0xffffffffffffc120])) [233 S8 A8])) -1 > (expr_list:REG_ARGS_SIZE (const_int 0 [0]) > (nil))) > > Then peephole common code thinks it needs to re-create the REG_ARGS_SIZE > note and fails since the code is too complex for it to understand. (Which > is reasonable since it doesn't know what value is being restored from the > stack here.) > > However, the more fundamental problem seems to be that this transformation > should be invalid anyway, since it creates an intermediate state where > the stack pointer points to a location without proper backchain, which > violates the ABI. > > The following patch fixes this by disabling the fusion peephole in those > cases where it would introduce a new use of the stack pointer as temporary > register. > > Tested on powerpc64le-linux. OK for mainline (and 4.8 after the big patch > series is committed)? > > Bye, > Ulrich > > ChangeLog: > > * config/rs6000/rs6000.c (fusion_gpr_load_p): Refuse optimization > if it would clobber the stack pointer, even temporarily. Okay. Thanks, David
Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 208870) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -32519,6 +32519,11 @@ if (!peep2_reg_dead_p (2, addis_reg)) return false; + + /* If the target register being loaded is the stack pointer, we must + avoid loading any other value into it, even temporarily. */ + if (REG_P (target) && REGNO (target) == STACK_POINTER_REGNUM) + return false; } base_reg = XEXP (addr, 0);
Hello, when trying to build Ada for powerpc64le-linux, I ran into an ICE in fixup_args_size_notes. It turns out that the p8 fusion peephole acts on these two insns from the epilog sequence: (insn 1693 1078 1079 91 (set (reg:DI 7 7) (plus:DI (reg/f:DI 31 31) (const_int 65536 [0x10000]))) 82 {*adddi3_internal1} (nil)) (insn 1079 1693 1511 91 (set (reg/f:DI 1 1) (mem/c:DI (plus:DI (reg:DI 7 7) (const_int -16096 [0xffffffffffffc120])) [233 %sfp+49440 S8 A64])) 519 {*movdi_internal64} (expr_list:REG_DEAD (reg:DI 7 7) (expr_list:REG_ARGS_SIZE (const_int 0 [0]) (nil)))) and replaces them by: (insn 1776 1078 1777 91 (set (reg/f:DI 1 1) (plus:DI (reg/f:DI 31 31) (const_int 65536 [0x10000]))) -1 (nil)) (insn 1777 1776 1511 91 (set (reg/f:DI 1 1) (mem/c:DI (plus:DI (reg/f:DI 1 1) (const_int -16096 [0xffffffffffffc120])) [233 S8 A8])) -1 (expr_list:REG_ARGS_SIZE (const_int 0 [0]) (nil))) Then peephole common code thinks it needs to re-create the REG_ARGS_SIZE note and fails since the code is too complex for it to understand. (Which is reasonable since it doesn't know what value is being restored from the stack here.) However, the more fundamental problem seems to be that this transformation should be invalid anyway, since it creates an intermediate state where the stack pointer points to a location without proper backchain, which violates the ABI. The following patch fixes this by disabling the fusion peephole in those cases where it would introduce a new use of the stack pointer as temporary register. Tested on powerpc64le-linux. OK for mainline (and 4.8 after the big patch series is committed)? Bye, Ulrich ChangeLog: * config/rs6000/rs6000.c (fusion_gpr_load_p): Refuse optimization if it would clobber the stack pointer, even temporarily.