Hi!
On Fri, Apr 17, 2020 at 01:35:15PM +0800, luoxhu wrote:
> On 2020/4/17 08:52, Segher Boessenkool wrote:
> > On Mon, Apr 13, 2020 at 10:11:43AM +0800, luoxhu wrote:
> >> frame_pointer_needed is set to true in reload pass setup_can_eliminate,
> >> but regs_ever_live[31] is false, pro_and_epilogue uses it without live
> >> check causing CPU2006 465.tonto segment fault of loading from invalid
> >> addresses due to r31 not saved/restored. Thus, add
> >> HARD_FRAME_POINTER_REGNUM
> >> live check with frame_pointer_needed_indeed_p when generating
> >> pro_and_epilogue
> >> instructions.
> >
> > I see.
> >
> > Can you instead make a boolean variable "frame_pointer_needed_indeed",
> > that you set somewhere early in *logue processing? So that we can be
> > sure that it will not change behind our backs.
>
>
> Thanks, rs6000_emit_prologue seems the proper place to set the
> frame_pointer_needed_indeed,
Yeah.
> but it's strange that hard_frame_pointer_rtx will be marked USE in
> make_prologue_seq, also
This is for when profiling is enabled. If this routine itself does not
use the hard frame pointer, the profiling code (not generated by GCC
here!) could still use it, so we need the USE to make sure later GCC
passes will see the hard frame pointer as live.
This is generic code; it isn't relevant for rs6000 I think.
> need check here though not causing segfault?
I'm not sure how it would?
> function.c
> static rtx_insn *
> make_prologue_seq (void)
> {
> if (!targetm.have_prologue ())
> return NULL;
>
> start_sequence ();
> rtx_insn *seq = targetm.gen_prologue ();
> emit_insn (seq);
>
> /* Insert an explicit USE for the frame pointer
> if the profiling is on and the frame pointer is required. */
> if (crtl->profile && frame_pointer_needed)
> emit_use (hard_frame_pointer_rtx);
> ...
> Any way, update the patch as below with your previous comments:
>
>
>
> This 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, pro_and_epilogue uses it without live
> check causing CPU2006 465.tonto segment fault of loading from invalid
> addresses due to r31 not saved/restored. Thus, add HARD_FRAME_POINTER_REGNUM
> live check with frame_pointer_needed_indeed when generating pro_and_epilogue
> instructions.
>
> Bootstrap and regression tested pass on Power8-LE. Backport to gcc-9
> required once approved.
>
> gcc/ChangeLog
>
> 2020-04-17 Xiong Hu Luo <luo...@linux.ibm.com>
>
> PR target/91518
> * config/rs6000/rs6000-logue.c (frame_pointer_needed_indeed):
> New variable.
> (rs6000_emit_prologue_components):
> Check with frame_pointer_needed_indeed.
> (rs6000_emit_epilogue_components): Likewise.
> (rs6000_emit_epilogue): Likewise.
> (rs6000_emit_prologue): Set frame_pointer_needed_indeed.
The patch is okay for trunk. (And for backports later). Thank you!
Segher
