| Message ID | 20200326041522.70199-1-luoxhu@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | rs6000: Save/restore r31 if frame_pointer_needed is true | expand |
On Wed, 2020-03-25 at 23:15 -0500, luoxhu--- via Gcc-patches wrote: > From: Xionghu Luo <luoxhu@linux.vnet.ibm.com> > Hi, No real issues noted in my review. Patch is straighforward, just a couple cosmetic comments inline below. > This P1 bug is exposed by FRE refactor of r263875. Comparing the fre > dump file shows no obvious change of the segment fault function > proves > it to be a target issue. > frame_pointer_needed is set to true in reload pass > setup_can_eliminate, > but regs_ever_live[31] is false, so pro_and_epilogue doesn't > save/restore > r31 even it is used actually, causing CPU2006 465.tonto segment fault > of > loading from invalid addresses. Could also use a statement here that also reflects what the patch does. "Thus, mark the register as in-use if frame_pointer_needed is true and reg is HARD_FRAME_POINTER_REGNUM." > Bootstrap and regression tested pass on Power8-LE. Backport to gcc-9 > required once approved. > > gcc/ChangeLog > > 2020-03-26 Xiong Hu Luo <luoxhu@linux.ibm.com> > > PR target/91518 > * config/rs6000/rs6000-logue.c (save_reg_p): Save r31 if > frame_pointer_needed. We don't actually reference 'r31' in the code change. Maybe preferred to refer to it as HARD_FRAME_POINTER_REGNUM instead? > --- > gcc/config/rs6000/rs6000-logue.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/gcc/config/rs6000/rs6000-logue.c > b/gcc/config/rs6000/rs6000-logue.c > index 4cbf228eb79..7b62ff03afd 100644 > --- a/gcc/config/rs6000/rs6000-logue.c > +++ b/gcc/config/rs6000/rs6000-logue.c > @@ -116,6 +116,9 @@ save_reg_p (int reg) > return true; > } > > + if (reg == HARD_FRAME_POINTER_REGNUM && frame_pointer_needed) > + return true; > + Ok. > return !call_used_or_fixed_reg_p (reg) && df_regs_ever_live_p > (reg); > } Thanks -Will
Hi! On Wed, Mar 25, 2020 at 11:15:22PM -0500, luoxhu@linux.ibm.com wrote: > frame_pointer_needed is set to true in reload pass setup_can_eliminate, > but regs_ever_live[31] is false, so pro_and_epilogue doesn't save/restore > r31 even it is used actually, causing CPU2006 465.tonto segment fault of > loading from invalid addresses. If df_regs_ever_live_p(31) is false there is no hard frame pointer anywhere in the program. How can it be used then? Segher
On 2020/3/27 07:59, Segher Boessenkool wrote: > Hi! > > On Wed, Mar 25, 2020 at 11:15:22PM -0500, luoxhu@linux.ibm.com wrote: >> frame_pointer_needed is set to true in reload pass setup_can_eliminate, >> but regs_ever_live[31] is false, so pro_and_epilogue doesn't save/restore >> r31 even it is used actually, causing CPU2006 465.tonto segment fault of >> loading from invalid addresses. > > If df_regs_ever_live_p(31) is false there is no hard frame pointer > anywhere in the program. How can it be used then? There is a piece of code emit move instruction to r31 even df_regs_ever_live_p(31) is false in pro_and_epilogue. As frame_point_needed is true and frame_pointer_needed is widely used in this function, so I propose to save r31 in save_reg_p instead of check (frame_pointer_needed && df_regs_ever_live_p(31), I haven't verify whether this works yet). Is this reasonable? Thanks. rs6000-logue.c void rs6000_emit_prologue (void) { ... bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26840) /* Set frame pointer, if needed. */ bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26841) if (frame_pointer_needed) bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26842) { 0d6c02bf24e4 (jakub 2005-06-30 14:26:32 +0000 26843) insn = emit_move_insn (gen_rtx_REG (Pmode, HARD_FRAME_POINTER_REGNUM), bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26844) sp_reg_rtx); bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26845) RTX_FRAME_RELATED_P (insn) = 1; 6b02f2a5c61e (meissner 1995-11-30 20:02:16 +0000 26846) } d1bd513ed578 (kenner 1992-02-09 19:26:21 +0000 26847) ... } Xionghu > > > Segher >
Hi! On Fri, Mar 27, 2020 at 09:34:00AM +0800, luoxhu wrote: > On 2020/3/27 07:59, Segher Boessenkool wrote: > > On Wed, Mar 25, 2020 at 11:15:22PM -0500, luoxhu@linux.ibm.com wrote: > >> frame_pointer_needed is set to true in reload pass setup_can_eliminate, > >> but regs_ever_live[31] is false, so pro_and_epilogue doesn't save/restore > >> r31 even it is used actually, causing CPU2006 465.tonto segment fault of > >> loading from invalid addresses. > > > > If df_regs_ever_live_p(31) is false there is no hard frame pointer > > anywhere in the program. How can it be used then? > > There is a piece of code emit move instruction to r31 even df_regs_ever_live_p(31) is false > in pro_and_epilogue. Can you point out where (in rs6000-logue.c ot similar)? We should fix *that*. > As frame_point_needed is true and frame_pointer_needed is widely > used in this function, so I propose to save r31 in save_reg_p instead of check > (frame_pointer_needed && df_regs_ever_live_p(31), I haven't verify whether this works yet). > Is this reasonable? Thanks. frame_pointer_needed is often true when the backend can figure out we do not actually need it. > rs6000-logue.c > void > rs6000_emit_prologue (void) > { > ... > bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26840) /* Set frame pointer, if needed. */ > bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26841) if (frame_pointer_needed) > bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26842) { > 0d6c02bf24e4 (jakub 2005-06-30 14:26:32 +0000 26843) insn = emit_move_insn (gen_rtx_REG (Pmode, HARD_FRAME_POINTER_REGNUM), > bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26844) sp_reg_rtx); > bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26845) RTX_FRAME_RELATED_P (insn) = 1; > 6b02f2a5c61e (meissner 1995-11-30 20:02:16 +0000 26846) } > d1bd513ed578 (kenner 1992-02-09 19:26:21 +0000 26847) > ... > } Ah, so this you mean. I see. It looks like if you change this to if (frame_pointer_needed && df_regs_ever_live_p (HARD_FRAME_POINTER_REGNUM)) { insn = emit_move_insn (gen_rtx_REG (Pmode, HARD_FRAME_POINTER_REGNUM), sp_reg_rtx); RTX_FRAME_RELATED_P (insn) = 1; } (so just that "if" clause changes), it'll all be fine. Could you test that please? Thanks, Segher
On 2020/3/28 00:04, Segher Boessenkool wrote: > Hi! > > On Fri, Mar 27, 2020 at 09:34:00AM +0800, luoxhu wrote: >> On 2020/3/27 07:59, Segher Boessenkool wrote: >>> On Wed, Mar 25, 2020 at 11:15:22PM -0500, luoxhu@linux.ibm.com wrote: >>>> frame_pointer_needed is set to true in reload pass setup_can_eliminate, >>>> but regs_ever_live[31] is false, so pro_and_epilogue doesn't save/restore >>>> r31 even it is used actually, causing CPU2006 465.tonto segment fault of >>>> loading from invalid addresses. >>> >>> If df_regs_ever_live_p(31) is false there is no hard frame pointer >>> anywhere in the program. How can it be used then? >> >> There is a piece of code emit move instruction to r31 even df_regs_ever_live_p(31) is false >> in pro_and_epilogue. > > Can you point out where (in rs6000-logue.c ot similar)? We should fix > *that*. > >> As frame_point_needed is true and frame_pointer_needed is widely >> used in this function, so I propose to save r31 in save_reg_p instead of check >> (frame_pointer_needed && df_regs_ever_live_p(31), I haven't verify whether this works yet). >> Is this reasonable? Thanks. > > frame_pointer_needed is often true when the backend can figure out we do > not actually need it. > >> rs6000-logue.c >> void >> rs6000_emit_prologue (void) >> { >> ... >> bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26840) /* Set frame pointer, if needed. */ >> bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26841) if (frame_pointer_needed) >> bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26842) { >> 0d6c02bf24e4 (jakub 2005-06-30 14:26:32 +0000 26843) insn = emit_move_insn (gen_rtx_REG (Pmode, HARD_FRAME_POINTER_REGNUM), >> bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26844) sp_reg_rtx); >> bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26845) RTX_FRAME_RELATED_P (insn) = 1; >> 6b02f2a5c61e (meissner 1995-11-30 20:02:16 +0000 26846) } >> d1bd513ed578 (kenner 1992-02-09 19:26:21 +0000 26847) >> ... >> } > > Ah, so this you mean. I see. It looks like if you change this to > > if (frame_pointer_needed && df_regs_ever_live_p (HARD_FRAME_POINTER_REGNUM)) > { > insn = emit_move_insn (gen_rtx_REG (Pmode, HARD_FRAME_POINTER_REGNUM), > sp_reg_rtx); > RTX_FRAME_RELATED_P (insn) = 1; > } > > (so just that "if" clause changes), it'll all be fine. Could you test > that please? Tried with below patch, it still fails at same place, I guess this is not enough. The instruction in 100dc034 change r31 from 0x105fdd70 to 0x7fffffffbbf0 and didn't restore it before return. 00000000100dc020 <__atomvec_module_MOD_atom_index_from_pos>: 100dc020: 51 10 40 3c lis r2,4177 100dc024: 00 7f 42 38 addi r2,r2,32512 100dc028: 28 00 e3 e8 ld r7,40(r3) 100dc02c: e1 ff 21 f8 stdu r1,-32(r1) 100dc030: 00 00 27 2c cmpdi r7,0 100dc034: 78 0b 3f 7c mr r31,r1 100dc038: 08 00 82 40 bne 100dc040 <__atomvec_module_MOD_atom_index_from_pos+0x20> 100dc03c: 01 00 e0 38 li r7,1 100dc040: 38 00 43 e9 ld r10,56(r3) 100dc044: 30 00 23 e9 ld r9,48(r3) 100dc048: 00 00 03 e9 ld r8,0(r3) Diff: diff --git a/gcc/config/rs6000/rs6000-logue.c b/gcc/config/rs6000/rs6000-logue.c index 4cbf228eb79..28fda1866d8 100644 --- a/gcc/config/rs6000/rs6000-logue.c +++ b/gcc/config/rs6000/rs6000-logue.c @@ -3658,7 +3658,7 @@ rs6000_emit_prologue (void) } /* Set frame pointer, if needed. */ - if (frame_pointer_needed) + if (frame_pointer_needed && df_regs_ever_live_p (HARD_FRAME_POINTER_REGNUM)) { insn = emit_move_insn (gen_rtx_REG (Pmode, HARD_FRAME_POINTER_REGNUM), sp_reg_rtx); @@ -4534,7 +4534,8 @@ rs6000_emit_epilogue (enum epilogue_type epilogue_type) } /* If we have a frame pointer, we can restore the old stack pointer from it. */ - else if (frame_pointer_needed) + else if (frame_pointer_needed + && df_regs_ever_live_p (HARD_FRAME_POINTER_REGNUM)) { frame_reg_rtx = sp_reg_rtx; if (DEFAULT_ABI == ABI_V4) @@ -4873,7 +4874,8 @@ rs6000_emit_epilogue (enum epilogue_type epilogue_type) a REG_CFA_DEF_CFA note, but that's OK; A duplicate is discarded by dwarf2cfi.c/dwarf2out.c, and in any case would be harmless if emitted. */ - if (frame_pointer_needed) + if (frame_pointer_needed + && df_regs_ever_live_p (HARD_FRAME_POINTER_REGNUM)) { insn = get_last_insn (); add_reg_note (insn, REG_CFA_DEF_CFA, Program received signal SIGSEGV: Segmentation fault - invalid memory reference. Backtrace for this error: #0 0x2000000a2243 in ??? #1 0x2000000a0abb in ??? #2 0x2000000604d7 in ??? #3 0x1027418c in __mol_module_MOD_make_image_of_shell at /home/luoxhu/workspace/cpu2006/benchspec/CPU2006/465.tonto/build/build_peak_none.0000/mol.fppized.f90:9376 #4 0x10281adf in __mol_module_MOD_symmetrise_r at /home/luoxhu/workspace/cpu2006/benchspec/CPU2006/465.tonto/build/build_peak_none.0000/mol.fppized.f90:9276 #5 0x10322d3b in __mol_module_MOD_symmetrise.constprop.0 at /home/luoxhu/workspace/cpu2006/benchspec/CPU2006/465.tonto/build/build_peak_none.0000/mol.fppized.f90:9239 #6 0x1023d1ff in __mol_module_MOD_make_atom_density at /home/luoxhu/workspace/cpu2006/benchspec/CPU2006/465.tonto/build/build_peak_none.0000/mol.fppized.f90:12477 #7 0x1023d9a3 in __mol_module_MOD_get_atom_density at /home/luoxhu/workspace/cpu2006/benchspec/CPU2006/465.tonto/build/build_peak_none.0000/mol.fppized.f90:12434 #8 0x1023e87b in __mol_module_MOD_make_atom_guess at /home/luoxhu/workspace/cpu2006/benchspec/CPU2006/465.tonto/build/build_peak_none.0000/mol.fppized.f90:12406 #9 0x1023e87b in __mol_module_MOD_get_initial_density Thanks, Xionghu > > Thanks, > > > Segher > 00000000100dc020 <__atomvec_module_MOD_atom_index_from_pos>: 100dc020: 51 10 40 3c lis r2,4177 100dc024: 00 7f 42 38 addi r2,r2,32512 100dc028: 28 00 e3 e8 ld r7,40(r3) 100dc02c: e1 ff 21 f8 stdu r1,-32(r1) 100dc030: 00 00 27 2c cmpdi r7,0 100dc034: 78 0b 3f 7c mr r31,r1 100dc038: 08 00 82 40 bne 100dc040 <__atomvec_module_MOD_atom_index_from_pos+0x20> 100dc03c: 01 00 e0 38 li r7,1 100dc040: 38 00 43 e9 ld r10,56(r3) 100dc044: 30 00 23 e9 ld r9,48(r3) 100dc048: 00 00 03 e9 ld r8,0(r3) 100dc04c: 50 50 69 7c subf r3,r9,r10 100dc050: 01 00 03 38 addi r0,r3,1 100dc054: f8 00 05 7c not r5,r0 100dc058: 76 fe a6 7c sradi r6,r5,63 100dc05c: 38 30 0b 7c and r11,r0,r6 100dc060: 00 00 8b 2c cmpwi cr1,r11,0 100dc064: b4 07 6c 7d extsw r12,r11 100dc068: 58 03 85 40 ble cr1,100dc3c0 <__atomvec_module_MOD_atom_index_from_pos+0x3a0> 100dc06c: 88 00 28 39 addi r9,r8,136 100dc070: 98 26 00 7c lxvd2x vs0,0,r4 100dc074: f7 ff 42 3d addis r10,r2,-9 100dc078: 10 00 84 c9 lfd f12,16(r4) 100dc07c: 10 00 69 c9 lfd f11,16(r9) 100dc080: ff ff 0c 39 addi r8,r12,-1 100dc084: e0 04 e7 1c mulli r7,r7,1248 100dc088: 98 4e 40 7d lxvd2x vs10,0,r9 100dc08c: c0 dc 2a c9 lfd f9,-9024(r10) 100dc090: be 07 00 55 clrlwi r0,r8,30 100dc094: 01 00 60 38 li r3,1 100dc098: 78 0b 26 7c mr r6,r1 100dc09c: 28 58 4c fc fsub f2,f12,f11 100dc0a0: 40 53 20 f0 xvsubdp vs1,vs0,vs10 100dc0a4: 10 12 80 fc fabs f4,f2 100dc0a8: 64 0f 60 f0 xvabsdp vs3,vs1 100dc0ac: 90 18 a0 fc fmr f5,f3 100dc0b0: 00 48 85 fe fcmpu cr5,f5,f9 100dc0b4: 30 03 95 41 bgt cr5,100dc3e4 <__atomvec_module_MOD_atom_index_from_pos+0x3c4> 100dc0b8: 50 1b c3 f0 xxspltd vs6,vs3,1 100dc0bc: 00 48 06 ff fcmpu cr6,f6,f9 100dc0c0: 24 03 99 41 bgt cr6,100dc3e4 <__atomvec_module_MOD_atom_index_from_pos+0x3c4> 100dc0c4: 00 48 84 ff fcmpu cr7,f4,f9 100dc0c8: 1c 03 9d 41 bgt cr7,100dc3e4 <__atomvec_module_MOD_atom_index_from_pos+0x3c4> 100dc0cc: b2 01 e6 fc fmul f7,f6,f6 100dc0d0: f8 ff a2 3c addis r5,r2,-8 100dc0d4: d0 eb 05 c9 lfd f8,-5168(r5) 100dc0d8: 7a 39 a5 fd fmadd f13,f5,f5,f7 100dc0dc: ba 68 02 fc fmadd f0,f2,f2,f13 100dc0e0: 00 40 00 fc fcmpu cr0,f0,f8 100dc0e4: 1c 03 80 41 blt 100dc400 <__atomvec_module_MOD_atom_index_from_pos+0x3e0> 100dc0e8: 01 00 8c 2c cmpwi cr1,r12,1 100dc0ec: 14 3a 09 7d add r8,r9,r7 100dc0f0: 02 00 60 38 li r3,2 100dc0f4: cc 02 85 40 ble cr1,100dc3c0 <__atomvec_module_MOD_atom_index_from_pos+0x3a0> 100dc0f8: 00 00 80 2e cmpwi cr5,r0,0 100dc0fc: 60 01 96 41 beq cr5,100dc25c <__atomvec_module_MOD_atom_index_from_pos+0x23c> 100dc100: 01 00 00 2f cmpwi cr6,r0,1 100dc104: e4 00 9a 41 beq cr6,100dc1e8 <__atomvec_module_MOD_atom_index_from_pos+0x1c8> 100dc108: 02 00 80 2f cmpwi cr7,r0,2 100dc10c: 70 00 9e 41 beq cr7,100dc17c <__atomvec_module_MOD_atom_index_from_pos+0x15c> 100dc110: 98 26 20 7c lxvd2x vs1,0,r4 100dc114: 98 46 40 7d lxvd2x vs10,0,r8 100dc118: 78 0b 2b 7c mr r11,r1 100dc11c: 10 00 84 c9 lfd f12,16(r4) 100dc120: 10 00 68 c9 lfd f11,16(r8) 100dc124: 40 53 41 f0 xvsubdp vs2,vs1,vs10 100dc128: 28 58 6c fc fsub f3,f12,f11 100dc12c: 64 17 80 f0 xvabsdp vs4,vs2 100dc130: 10 1a a0 fc fabs f5,f3 100dc134: 90 20 c0 fc fmr f6,f4 100dc138: 00 48 06 fc fcmpu cr0,f6,f9 100dc13c: 0c 03 81 41 bgt 100dc448 <__atomvec_module_MOD_atom_index_from_pos+0x428> 100dc140: 50 23 e4 f0 xxspltd vs7,vs4,1 100dc144: 00 48 87 fc fcmpu cr1,f7,f9 100dc148: 00 03 85 41 bgt cr1,100dc448 <__atomvec_module_MOD_atom_index_from_pos+0x428> 100dc14c: 00 48 85 fe fcmpu cr5,f5,f9 100dc150: f8 02 95 41 bgt cr5,100dc448 <__atomvec_module_MOD_atom_index_from_pos+0x428> 100dc154: f2 01 07 fd fmul f8,f7,f7 100dc158: f8 ff 22 3d addis r9,r2,-8 100dc15c: d0 eb a9 c9 lfd f13,-5168(r9) 100dc160: ba 41 06 fc fmadd f0,f6,f6,f8 100dc164: fa 00 23 fc fmadd f1,f3,f3,f0 100dc168: 00 68 01 ff fcmpu cr6,f1,f13 100dc16c: 94 02 98 41 blt cr6,100dc400 <__atomvec_module_MOD_atom_index_from_pos+0x3e0> 100dc170: 01 00 03 38 addi r0,r3,1 100dc174: 14 3a 08 7d add r8,r8,r7 100dc178: b4 07 03 7c extsw r3,r0 100dc17c: 98 26 40 7c lxvd2x vs2,0,r4 100dc180: 98 46 40 7d lxvd2x vs10,0,r8 100dc184: 78 0b 25 7c mr r5,r1 100dc188: 10 00 84 c9 lfd f12,16(r4) 100dc18c: 10 00 68 c9 lfd f11,16(r8) 100dc190: 40 53 62 f0 xvsubdp vs3,vs2,vs10 100dc194: 28 58 8c fc fsub f4,f12,f11 100dc198: 64 1f a0 f0 xvabsdp vs5,vs3 100dc19c: 10 22 c0 fc fabs f6,f4 100dc1a0: 90 28 e0 fc fmr f7,f5 100dc1a4: 00 48 87 ff fcmpu cr7,f7,f9 100dc1a8: 90 02 9d 41 bgt cr7,100dc438 <__atomvec_module_MOD_atom_index_from_pos+0x418> 100dc1ac: 50 2b 05 f1 xxspltd vs8,vs5,1 100dc1b0: 00 48 08 fc fcmpu cr0,f8,f9 100dc1b4: 84 02 81 41 bgt 100dc438 <__atomvec_module_MOD_atom_index_from_pos+0x418> 100dc1b8: 00 48 86 fc fcmpu cr1,f6,f9 100dc1bc: 7c 02 85 41 bgt cr1,100dc438 <__atomvec_module_MOD_atom_index_from_pos+0x418> 100dc1c0: 32 02 a8 fd fmul f13,f8,f8 100dc1c4: f8 ff c2 3c addis r6,r2,-8 100dc1c8: d0 eb 26 c8 lfd f1,-5168(r6) 100dc1cc: fa 69 07 fc fmadd f0,f7,f7,f13 100dc1d0: 3a 01 44 fc fmadd f2,f4,f4,f0 100dc1d4: 00 08 82 fe fcmpu cr5,f2,f1 100dc1d8: 28 02 94 41 blt cr5,100dc400 <__atomvec_module_MOD_atom_index_from_pos+0x3e0> 100dc1dc: 01 00 63 38 addi r3,r3,1 100dc1e0: 14 3a 08 7d add r8,r8,r7 100dc1e4: b4 07 63 7c extsw r3,r3 100dc1e8: 98 26 80 7c lxvd2x vs4,0,r4 100dc1ec: 98 46 40 7d lxvd2x vs10,0,r8 100dc1f0: 78 0b 25 7c mr r5,r1 100dc1f4: 10 00 a4 c8 lfd f5,16(r4) 100dc1f8: 40 53 44 f1 xvsubdp vs10,vs4,vs10 100dc1fc: 10 00 68 c9 lfd f11,16(r8) 100dc200: 28 58 85 fd fsub f12,f5,f11 100dc204: 64 57 60 f0 xvabsdp vs3,vs10 100dc208: 10 62 80 fc fabs f4,f12 100dc20c: 90 18 a0 fc fmr f5,f3 100dc210: 00 48 05 ff fcmpu cr6,f5,f9 100dc214: 04 02 99 41 bgt cr6,100dc418 <__atomvec_module_MOD_atom_index_from_pos+0x3f8> 100dc218: 50 1b c3 f0 xxspltd vs6,vs3,1 100dc21c: 00 48 86 ff fcmpu cr7,f6,f9 100dc220: f8 01 9d 41 bgt cr7,100dc418 <__atomvec_module_MOD_atom_index_from_pos+0x3f8> 100dc224: 00 48 04 fc fcmpu cr0,f4,f9 100dc228: f0 01 81 41 bgt 100dc418 <__atomvec_module_MOD_atom_index_from_pos+0x3f8> 100dc22c: b2 01 e6 fc fmul f7,f6,f6 100dc230: f8 ff 22 3d addis r9,r2,-8 100dc234: d0 eb 09 c9 lfd f8,-5168(r9) 100dc238: 7a 39 a5 fd fmadd f13,f5,f5,f7 100dc23c: 3a 6b 2c fc fmadd f1,f12,f12,f13 100dc240: 00 40 81 fc fcmpu cr1,f1,f8 100dc244: bc 01 84 41 blt cr1,100dc400 <__atomvec_module_MOD_atom_index_from_pos+0x3e0> 100dc248: 01 00 43 39 addi r10,r3,1 100dc24c: 14 3a 08 7d add r8,r8,r7 100dc250: 00 60 8a 7e cmpw cr5,r10,r12 100dc254: b4 07 43 7d extsw r3,r10 100dc258: 68 01 95 41 bgt cr5,100dc3c0 <__atomvec_module_MOD_atom_index_from_pos+0x3a0> 100dc25c: 98 26 00 7c lxvd2x vs0,0,r4 100dc260: 98 46 40 7c lxvd2x vs2,0,r8 100dc264: 78 0b 25 7c mr r5,r1 100dc268: 10 00 44 c9 lfd f10,16(r4) 100dc26c: 10 00 68 c9 lfd f11,16(r8) 100dc270: 40 13 60 f0 xvsubdp vs3,vs0,vs2 100dc274: 28 58 8a fd fsub f12,f10,f11 100dc278: 64 1f 80 f0 xvabsdp vs4,vs3 100dc27c: 10 62 a0 fc fabs f5,f12 100dc280: 90 20 c0 fc fmr f6,f4 100dc284: 00 48 06 ff fcmpu cr6,f6,f9 100dc288: 80 01 99 41 bgt cr6,100dc408 <__atomvec_module_MOD_atom_index_from_pos+0x3e8> 100dc28c: 50 23 e4 f0 xxspltd vs7,vs4,1 100dc290: 00 48 87 ff fcmpu cr7,f7,f9 100dc294: 74 01 9d 41 bgt cr7,100dc408 <__atomvec_module_MOD_atom_index_from_pos+0x3e8> 100dc298: 00 48 05 fc fcmpu cr0,f5,f9 100dc29c: 6c 01 81 41 bgt 100dc408 <__atomvec_module_MOD_atom_index_from_pos+0x3e8> 100dc2a0: f2 01 07 fd fmul f8,f7,f7 100dc2a4: f8 ff c2 3c addis r6,r2,-8 100dc2a8: d0 eb a6 c9 lfd f13,-5168(r6) 100dc2ac: ba 41 26 fc fmadd f1,f6,f6,f8 100dc2b0: 3a 0b 0c fc fmadd f0,f12,f12,f1 100dc2b4: 00 68 80 fc fcmpu cr1,f0,f13 100dc2b8: 48 01 84 41 blt cr1,100dc400 <__atomvec_module_MOD_atom_index_from_pos+0x3e0> 100dc2bc: 98 26 40 7c lxvd2x vs2,0,r4 100dc2c0: 98 3e 48 7d lxvd2x vs10,r8,r7 100dc2c4: 14 3a 28 7d add r9,r8,r7 100dc2c8: 01 00 63 38 addi r3,r3,1 100dc2cc: 10 00 64 c8 lfd f3,16(r4) 100dc2d0: b4 07 63 7c extsw r3,r3 100dc2d4: 78 0b 2a 7c mr r10,r1 100dc2d8: 10 00 69 c9 lfd f11,16(r9) 100dc2dc: 40 53 82 f0 xvsubdp vs4,vs2,vs10 100dc2e0: 28 58 83 fd fsub f12,f3,f11 100dc2e4: 64 27 a0 f0 xvabsdp vs5,vs4 100dc2e8: 10 62 c0 fc fabs f6,f12 100dc2ec: 90 28 e0 fc fmr f7,f5 100dc2f0: 00 48 87 fe fcmpu cr5,f7,f9 100dc2f4: 34 01 95 41 bgt cr5,100dc428 <__atomvec_module_MOD_atom_index_from_pos+0x408> 100dc2f8: 50 2b 05 f1 xxspltd vs8,vs5,1 100dc2fc: 00 48 08 ff fcmpu cr6,f8,f9 100dc300: 28 01 99 41 bgt cr6,100dc428 <__atomvec_module_MOD_atom_index_from_pos+0x408> 100dc304: 00 48 86 ff fcmpu cr7,f6,f9 100dc308: 20 01 9d 41 bgt cr7,100dc428 <__atomvec_module_MOD_atom_index_from_pos+0x408> 100dc30c: 32 02 a8 fd fmul f13,f8,f8 100dc310: f8 ff 02 3d addis r8,r2,-8 100dc314: d0 eb 28 c8 lfd f1,-5168(r8) 100dc318: fa 69 07 fc fmadd f0,f7,f7,f13 100dc31c: 3a 03 4c fc fmadd f2,f12,f12,f0 100dc320: 00 08 02 fc fcmpu cr0,f2,f1 100dc324: dc 00 80 41 blt 100dc400 <__atomvec_module_MOD_atom_index_from_pos+0x3e0> 100dc328: 98 26 60 7c lxvd2x vs3,0,r4 100dc32c: 98 3e 49 7d lxvd2x vs10,r9,r7 100dc330: 14 3a a9 7c add r5,r9,r7 100dc334: 01 00 c3 38 addi r6,r3,1 100dc338: 10 00 84 c8 lfd f4,16(r4) 100dc33c: b4 07 c3 7c extsw r3,r6 100dc340: 78 0b 2b 7c mr r11,r1 100dc344: 10 00 65 c9 lfd f11,16(r5) 100dc348: 40 53 a3 f0 xvsubdp vs5,vs3,vs10 100dc34c: 28 58 84 fd fsub f12,f4,f11 100dc350: 64 2f c0 f0 xvabsdp vs6,vs5 100dc354: 10 62 e0 fc fabs f7,f12 100dc358: 90 30 00 fd fmr f8,f6 100dc35c: 00 48 88 fc fcmpu cr1,f8,f9 100dc360: f8 00 85 41 bgt cr1,100dc458 <__atomvec_module_MOD_atom_index_from_pos+0x438> 100dc364: 50 33 a6 f1 xxspltd vs13,vs6,1 100dc368: 00 48 8d fe fcmpu cr5,f13,f9 100dc36c: ec 00 95 41 bgt cr5,100dc458 <__atomvec_module_MOD_atom_index_from_pos+0x438> 100dc370: 00 48 07 ff fcmpu cr6,f7,f9 100dc374: e4 00 99 41 bgt cr6,100dc458 <__atomvec_module_MOD_atom_index_from_pos+0x438> 100dc378: 72 03 2d fc fmul f1,f13,f13 100dc37c: f8 ff 22 3d addis r9,r2,-8 100dc380: d0 eb 49 c8 lfd f2,-5168(r9) 100dc384: 3a 0a 08 fc fmadd f0,f8,f8,f1 100dc388: 3a 03 6c fc fmadd f3,f12,f12,f0 100dc38c: 00 10 83 ff fcmpu cr7,f3,f2 100dc390: 70 00 9c 41 blt cr7,100dc400 <__atomvec_module_MOD_atom_index_from_pos+0x3e0> 100dc394: 01 00 63 38 addi r3,r3,1 100dc398: 98 3e 45 7d lxvd2x vs10,r5,r7 100dc39c: 14 3a 05 7d add r8,r5,r7 100dc3a0: 98 26 80 7c lxvd2x vs4,0,r4 100dc3a4: 10 00 a4 c8 lfd f5,16(r4) 100dc3a8: b4 07 63 7c extsw r3,r3 100dc3ac: 78 0b 25 7c mr r5,r1 100dc3b0: 48 fe ff 4b b 100dc1f8 <__atomvec_module_MOD_atom_index_from_pos+0x1d8> 100dc3b4: 00 00 00 60 nop 100dc3b8: 00 00 00 60 nop 100dc3bc: 00 00 42 60 ori r2,r2,0 100dc3c0: a6 02 08 7c mflr r0 100dc3c4: f8 ff 82 3c addis r4,r2,-8 100dc3c8: 00 00 00 60 nop 100dc3cc: 39 00 a0 38 li r5,57 100dc3d0: 70 e7 84 38 addi r4,r4,-6288 100dc3d4: 80 83 62 38 addi r3,r2,-31872 100dc3d8: 30 00 1f f8 std r0,48(r31) 100dc3dc: 5d 66 39 48 bl 10472a38 <__system_module_MOD_die.constprop.0+0x8> 100dc3e0: 00 00 00 60 nop 100dc3e4: 00 00 61 e8 ld r3,0(r1) 100dc3e8: 00 00 66 f8 std r3,0(r6) 100dc3ec: 78 33 c1 7c mr r1,r6 100dc3f0: f8 fc ff 4b b 100dc0e8 <__atomvec_module_MOD_atom_index_from_pos+0xc8> 100dc3f4: 00 00 00 60 nop 100dc3f8: 00 00 00 60 nop 100dc3fc: 00 00 42 60 ori r2,r2,0 100dc400: 20 00 3f 38 addi r1,r31,32 100dc404: 20 00 80 4e blr 100dc408: 00 00 61 e9 ld r11,0(r1) 100dc40c: 00 00 65 f9 std r11,0(r5) 100dc410: 78 2b a1 7c mr r1,r5 100dc414: a8 fe ff 4b b 100dc2bc <__atomvec_module_MOD_atom_index_from_pos+0x29c> 100dc418: 00 00 01 e8 ld r0,0(r1) 100dc41c: 00 00 05 f8 std r0,0(r5) 100dc420: 78 2b a1 7c mr r1,r5 100dc424: 24 fe ff 4b b 100dc248 <__atomvec_module_MOD_atom_index_from_pos+0x228> 100dc428: 00 00 01 e8 ld r0,0(r1) 100dc42c: 00 00 0a f8 std r0,0(r10) 100dc430: 78 53 41 7d mr r1,r10 100dc434: f4 fe ff 4b b 100dc328 <__atomvec_module_MOD_atom_index_from_pos+0x308> 100dc438: 00 00 61 e9 ld r11,0(r1) 100dc43c: 00 00 65 f9 std r11,0(r5) 100dc440: 78 2b a1 7c mr r1,r5 100dc444: 98 fd ff 4b b 100dc1dc <__atomvec_module_MOD_atom_index_from_pos+0x1bc> 100dc448: 00 00 41 e9 ld r10,0(r1) 100dc44c: 00 00 4b f9 std r10,0(r11) 100dc450: 78 5b 61 7d mr r1,r11 100dc454: 1c fd ff 4b b 100dc170 <__atomvec_module_MOD_atom_index_from_pos+0x150> 100dc458: 00 00 41 e9 ld r10,0(r1) 100dc45c: 00 00 4b f9 std r10,0(r11) 100dc460: 78 5b 61 7d mr r1,r11 100dc464: 30 ff ff 4b b 100dc394 <__atomvec_module_MOD_atom_index_from_pos+0x374> 100dc468: 00 00 00 00 .long 0x0 100dc46c: 00 00 00 01 .long 0x1000000 100dc470: 80 01 00 00 .long 0x180 100dc474: 00 00 00 60 nop 100dc478: 00 00 00 60 nop 100dc47c: 00 00 42 60 ori r2,r2,0
On 3/30/20 12:18 AM, luoxhu via Gcc-patches wrote: > > > On 2020/3/28 00:04, Segher Boessenkool wrote: >> Hi! >> >> On Fri, Mar 27, 2020 at 09:34:00AM +0800, luoxhu wrote: >>> On 2020/3/27 07:59, Segher Boessenkool wrote: >>>> On Wed, Mar 25, 2020 at 11:15:22PM -0500, luoxhu@linux.ibm.com wrote: >>>>> frame_pointer_needed is set to true in reload pass >>>>> setup_can_eliminate, >>>>> but regs_ever_live[31] is false, so pro_and_epilogue doesn't >>>>> save/restore >>>>> r31 even it is used actually, causing CPU2006 465.tonto segment >>>>> fault of >>>>> loading from invalid addresses. >>>> >>>> If df_regs_ever_live_p(31) is false there is no hard frame pointer >>>> anywhere in the program. How can it be used then? >>> >>> There is a piece of code emit move instruction to r31 even >>> df_regs_ever_live_p(31) is false >>> in pro_and_epilogue. >> >> Can you point out where (in rs6000-logue.c ot similar)? We should fix >> *that*. >> >>> As frame_point_needed is true and frame_pointer_needed is widely >>> used in this function, so I propose to save r31 in save_reg_p >>> instead of check >>> (frame_pointer_needed && df_regs_ever_live_p(31), I haven't verify >>> whether this works yet). >>> Is this reasonable? Thanks. >> >> frame_pointer_needed is often true when the backend can figure out we do >> not actually need it. >> >>> rs6000-logue.c >>> void >>> rs6000_emit_prologue (void) >>> { >>> ... >>> bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26840) /* >>> Set frame pointer, if needed. */ >>> bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26841) if >>> (frame_pointer_needed) >>> bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26842) { >>> 0d6c02bf24e4 (jakub 2005-06-30 14:26:32 +0000 26843) >>> insn = emit_move_insn (gen_rtx_REG (Pmode, HARD_FRAME_POINTER_REGNUM), >>> bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 >>> 26844) sp_reg_rtx); >>> bbd21807fdf6 (geoffk 2000-03-16 03:16:41 +0000 26845) >>> RTX_FRAME_RELATED_P (insn) = 1; >>> 6b02f2a5c61e (meissner 1995-11-30 20:02:16 +0000 26846) } >>> d1bd513ed578 (kenner 1992-02-09 19:26:21 +0000 26847) >>> ... >>> } >> >> Ah, so this you mean. I see. It looks like if you change this to >> >> if (frame_pointer_needed && df_regs_ever_live_p >> (HARD_FRAME_POINTER_REGNUM)) >> { >> insn = emit_move_insn (gen_rtx_REG (Pmode, >> HARD_FRAME_POINTER_REGNUM), >> sp_reg_rtx); >> RTX_FRAME_RELATED_P (insn) = 1; >> } >> >> (so just that "if" clause changes), it'll all be fine. Could you test >> that please? > > Tried with below patch, it still fails at same place, I guess this is > not enough. > The instruction in 100dc034 change r31 from 0x105fdd70 to > 0x7fffffffbbf0 and didn't > restore it before return. > 00000000100dc020 <__atomvec_module_MOD_atom_index_from_pos>: > 100dc020: 51 10 40 3c lis r2,4177 > 100dc024: 00 7f 42 38 addi r2,r2,32512 > 100dc028: 28 00 e3 e8 ld r7,40(r3) > 100dc02c: e1 ff 21 f8 stdu r1,-32(r1) > 100dc030: 00 00 27 2c cmpdi r7,0 > 100dc034: 78 0b 3f 7c mr r31,r1 > 100dc038: 08 00 82 40 bne 100dc040 > <__atomvec_module_MOD_atom_index_from_pos+0x20> > 100dc03c: 01 00 e0 38 li r7,1 > 100dc040: 38 00 43 e9 ld r10,56(r3) > 100dc044: 30 00 23 e9 ld r9,48(r3) > 100dc048: 00 00 03 e9 ld r8,0(r3) > > > Diff: > > diff --git a/gcc/config/rs6000/rs6000-logue.c > b/gcc/config/rs6000/rs6000-logue.c > index 4cbf228eb79..28fda1866d8 100644 > --- a/gcc/config/rs6000/rs6000-logue.c > +++ b/gcc/config/rs6000/rs6000-logue.c > @@ -3658,7 +3658,7 @@ rs6000_emit_prologue (void) > } > > /* Set frame pointer, if needed. */ > - if (frame_pointer_needed) > + if (frame_pointer_needed && df_regs_ever_live_p > (HARD_FRAME_POINTER_REGNUM)) > { > insn = emit_move_insn (gen_rtx_REG (Pmode, > HARD_FRAME_POINTER_REGNUM), > sp_reg_rtx); > @@ -4534,7 +4534,8 @@ rs6000_emit_epilogue (enum epilogue_type > epilogue_type) > } > /* If we have a frame pointer, we can restore the old stack pointer > from it. */ > - else if (frame_pointer_needed) > + else if (frame_pointer_needed > + && df_regs_ever_live_p (HARD_FRAME_POINTER_REGNUM)) > { > frame_reg_rtx = sp_reg_rtx; > if (DEFAULT_ABI == ABI_V4) > @@ -4873,7 +4874,8 @@ rs6000_emit_epilogue (enum epilogue_type > epilogue_type) > a REG_CFA_DEF_CFA note, but that's OK; A duplicate is > discarded by dwarf2cfi.c/dwarf2out.c, and in any case would > be harmless if emitted. */ > - if (frame_pointer_needed) > + if (frame_pointer_needed > + && df_regs_ever_live_p (HARD_FRAME_POINTER_REGNUM)) > { > insn = get_last_insn (); > add_reg_note (insn, REG_CFA_DEF_CFA, > > > > Program received signal SIGSEGV: Segmentation fault - invalid memory > reference. > > Backtrace for this error: > #0 0x2000000a2243 in ??? > #1 0x2000000a0abb in ??? > #2 0x2000000604d7 in ??? > #3 0x1027418c in __mol_module_MOD_make_image_of_shell > at > /home/luoxhu/workspace/cpu2006/benchspec/CPU2006/465.tonto/build/build_peak_none.0000/mol.fppized.f90:9376 > #4 0x10281adf in __mol_module_MOD_symmetrise_r > at > /home/luoxhu/workspace/cpu2006/benchspec/CPU2006/465.tonto/build/build_peak_none.0000/mol.fppized.f90:9276 > #5 0x10322d3b in __mol_module_MOD_symmetrise.constprop.0 > at > /home/luoxhu/workspace/cpu2006/benchspec/CPU2006/465.tonto/build/build_peak_none.0000/mol.fppized.f90:9239 > #6 0x1023d1ff in __mol_module_MOD_make_atom_density > at > /home/luoxhu/workspace/cpu2006/benchspec/CPU2006/465.tonto/build/build_peak_none.0000/mol.fppized.f90:12477 > #7 0x1023d9a3 in __mol_module_MOD_get_atom_density > at > /home/luoxhu/workspace/cpu2006/benchspec/CPU2006/465.tonto/build/build_peak_none.0000/mol.fppized.f90:12434 > #8 0x1023e87b in __mol_module_MOD_make_atom_guess > at > /home/luoxhu/workspace/cpu2006/benchspec/CPU2006/465.tonto/build/build_peak_none.0000/mol.fppized.f90:12406 > #9 0x1023e87b in __mol_module_MOD_get_initial_density > > > Thanks, > Xionghu > Xionghu, I'm don't have access to SPEC to test this but on line 4534 in rs6000_emit_prologue in that if block: insn = emit_insn (gen_add3_insn (frame_reg_rtx, hard_frame_pointer_rtx, GEN_INT (info->total_size))); seems this should be an emit_move_insn and therefore probably just: insn = emit_move_insn (frame_reg_rtx, hard_frame_pointer_rtx); Seems to be what may be missing as the other callers all do emit_move_insn in your patch. Unless there is a edge case I'm missing. Cheers, Nick >> >> Thanks, >> >> >> Segher >>
Hi! On Mon, Mar 30, 2020 at 12:18:59PM +0800, luoxhu wrote: > >>>If df_regs_ever_live_p(31) is false there is no hard frame pointer > >>>anywhere in the program. How can it be used then? > >> > >>There is a piece of code emit move instruction to r31 even > >>df_regs_ever_live_p(31) is false > >>in pro_and_epilogue. > > > >Can you point out where (in rs6000-logue.c ot similar)? We should fix > >*that*. > >frame_pointer_needed is often true when the backend can figure out we do > >not actually need it. > >(so just that "if" clause changes), it'll all be fine. Could you test > >that please? > > Tried with below patch, it still fails at same place, I guess this is not > enough. > The instruction in 100dc034 change r31 from 0x105fdd70 to 0x7fffffffbbf0 > and didn't > restore it before return. So where was *that* insn generated, then? Segher
diff --git a/gcc/config/rs6000/rs6000-logue.c b/gcc/config/rs6000/rs6000-logue.c index 4cbf228eb79..7b62ff03afd 100644 --- a/gcc/config/rs6000/rs6000-logue.c +++ b/gcc/config/rs6000/rs6000-logue.c @@ -116,6 +116,9 @@ save_reg_p (int reg) return true; } + if (reg == HARD_FRAME_POINTER_REGNUM && frame_pointer_needed) + return true; + return !call_used_or_fixed_reg_p (reg) && df_regs_ever_live_p (reg); }
From: Xionghu Luo <luoxhu@linux.vnet.ibm.com> This P1 bug is exposed by FRE refactor of r263875. Comparing the fre dump file shows no obvious change of the segment fault function proves it to be a target issue. frame_pointer_needed is set to true in reload pass setup_can_eliminate, but regs_ever_live[31] is false, so pro_and_epilogue doesn't save/restore r31 even it is used actually, causing CPU2006 465.tonto segment fault of loading from invalid addresses. Bootstrap and regression tested pass on Power8-LE. Backport to gcc-9 required once approved. gcc/ChangeLog 2020-03-26 Xiong Hu Luo <luoxhu@linux.ibm.com> PR target/91518 * config/rs6000/rs6000-logue.c (save_reg_p): Save r31 if frame_pointer_needed. --- gcc/config/rs6000/rs6000-logue.c | 3 +++ 1 file changed, 3 insertions(+)