| Message ID | 20170812013228.GA3607@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Aug 12, 2017 at 3:32 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > When we eliminate frame pointer, we should also replace frame pointer > with stack pointer - UNITS_PER_WORD in debug insns. This patch fixed: > > FAIL: gcc.dg/guality/pr58791-5.c -Os line pr58791-5.c:20 b1 == 9 > FAIL: gcc.dg/guality/pr58791-5.c -Os line pr58791-5.c:20 b2 == 73 > FAIL: gcc.dg/guality/pr58791-5.c -Os line pr58791-5.c:20 b3 == 585 > FAIL: gcc.dg/guality/pr58791-5.c -Os line pr58791-5.c:20 b4 == 4681 > FAIL: gcc.dg/guality/pr59776.c -Os line pr59776.c:17 s1.f == 5.0 > FAIL: gcc.dg/guality/pr59776.c -Os line pr59776.c:17 s1.g == 6.0 > FAIL: gcc.dg/guality/pr59776.c -Os line pr59776.c:17 s2.g == 6.0 > FAIL: gcc.dg/guality/pr59776.c -Os line pr59776.c:20 s1.f == 5.0 > FAIL: gcc.dg/guality/pr59776.c -Os line pr59776.c:20 s1.g == 6.0 > FAIL: gcc.dg/guality/pr59776.c -Os line pr59776.c:20 s2.f == 5.0 > FAIL: gcc.dg/guality/pr59776.c -Os line pr59776.c:20 s2.g == 6.0 > > on Linux/i386. > > Tested on i686 and x86-64. OK for trunk? Is it possible to detect when frame pointer setup was omitted and do the replacement only in that case? Do we have to do the replacement unconditinally due to shrink-wrapping and similar passes? Uros. > H.J. > --- > > PR target/81820 > * config/i386/i386.c (ix86_finalize_stack_frame_flags): Replace > frame pointer with stack pointer - UNITS_PER_WORD in debug insns. > --- > gcc/config/i386/i386.c | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index b04321a8d40..0094f2c4441 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -14281,6 +14281,42 @@ ix86_finalize_stack_frame_flags (void) > df_scan_blocks (); > df_compute_regs_ever_live (true); > df_analyze (); > + > + if (flag_var_tracking) > + { > + /* Since frame pointer is no longer needed, replace it with > + stack pointer - UNITS_PER_WORD in debug insns. */ > + df_ref ref, next; > + for (ref = DF_REG_USE_CHAIN (HARD_FRAME_POINTER_REGNUM); > + ref; ref = next) > + { > + rtx_insn *insn = DF_REF_INSN (ref); > + /* Make sure the next ref is for a different instruction, > + so that we're not affected by the rescan. */ > + next = DF_REF_NEXT_REG (ref); > + while (next && DF_REF_INSN (next) == insn) > + next = DF_REF_NEXT_REG (next); > + > + if (DEBUG_INSN_P (insn)) > + { > + bool changed = false; > + for (; ref != next; ref = DF_REF_NEXT_REG (ref)) > + { > + rtx *loc = DF_REF_LOC (ref); > + if (*loc == hard_frame_pointer_rtx) > + { > + *loc = plus_constant (Pmode, > + stack_pointer_rtx, > + -UNITS_PER_WORD); > + changed = true; > + } > + } > + if (changed) > + df_insn_rescan (insn); > + } > + } > + } > + > recompute_frame_layout_p = true; > } > > -- > 2.13.4 >
On Sat, Aug 12, 2017 at 3:32 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > When we eliminate frame pointer, we should also replace frame pointer > with stack pointer - UNITS_PER_WORD in debug insns. This patch fixed: > > FAIL: gcc.dg/guality/pr58791-5.c -Os line pr58791-5.c:20 b1 == 9 > FAIL: gcc.dg/guality/pr58791-5.c -Os line pr58791-5.c:20 b2 == 73 > FAIL: gcc.dg/guality/pr58791-5.c -Os line pr58791-5.c:20 b3 == 585 > FAIL: gcc.dg/guality/pr58791-5.c -Os line pr58791-5.c:20 b4 == 4681 > FAIL: gcc.dg/guality/pr59776.c -Os line pr59776.c:17 s1.f == 5.0 > FAIL: gcc.dg/guality/pr59776.c -Os line pr59776.c:17 s1.g == 6.0 > FAIL: gcc.dg/guality/pr59776.c -Os line pr59776.c:17 s2.g == 6.0 > FAIL: gcc.dg/guality/pr59776.c -Os line pr59776.c:20 s1.f == 5.0 > FAIL: gcc.dg/guality/pr59776.c -Os line pr59776.c:20 s1.g == 6.0 > FAIL: gcc.dg/guality/pr59776.c -Os line pr59776.c:20 s2.f == 5.0 > FAIL: gcc.dg/guality/pr59776.c -Os line pr59776.c:20 s2.g == 6.0 > > on Linux/i386. > > Tested on i686 and x86-64. OK for trunk? > > H.J. > --- > > PR target/81820 > * config/i386/i386.c (ix86_finalize_stack_frame_flags): Replace > frame pointer with stack pointer - UNITS_PER_WORD in debug insns. > --- > gcc/config/i386/i386.c | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index b04321a8d40..0094f2c4441 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -14281,6 +14281,42 @@ ix86_finalize_stack_frame_flags (void) > df_scan_blocks (); > df_compute_regs_ever_live (true); > df_analyze (); > + > + if (flag_var_tracking) > + { > + /* Since frame pointer is no longer needed, replace it with > + stack pointer - UNITS_PER_WORD in debug insns. */ > + df_ref ref, next; > + for (ref = DF_REG_USE_CHAIN (HARD_FRAME_POINTER_REGNUM); > + ref; ref = next) > + { > + rtx_insn *insn = DF_REF_INSN (ref); > + /* Make sure the next ref is for a different instruction, > + so that we're not affected by the rescan. */ > + next = DF_REF_NEXT_REG (ref); > + while (next && DF_REF_INSN (next) == insn) > + next = DF_REF_NEXT_REG (next); Can you please explain why the above loop is needed? Thanks, Uros. > + if (DEBUG_INSN_P (insn)) > + { > + bool changed = false; > + for (; ref != next; ref = DF_REF_NEXT_REG (ref)) > + { > + rtx *loc = DF_REF_LOC (ref); > + if (*loc == hard_frame_pointer_rtx) > + { > + *loc = plus_constant (Pmode, > + stack_pointer_rtx, > + -UNITS_PER_WORD); > + changed = true; > + } > + } > + if (changed) > + df_insn_rescan (insn); > + } > + } > + } > + > recompute_frame_layout_p = true; > } > > -- > 2.13.4 >
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index b04321a8d40..0094f2c4441 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -14281,6 +14281,42 @@ ix86_finalize_stack_frame_flags (void) df_scan_blocks (); df_compute_regs_ever_live (true); df_analyze (); + + if (flag_var_tracking) + { + /* Since frame pointer is no longer needed, replace it with + stack pointer - UNITS_PER_WORD in debug insns. */ + df_ref ref, next; + for (ref = DF_REG_USE_CHAIN (HARD_FRAME_POINTER_REGNUM); + ref; ref = next) + { + rtx_insn *insn = DF_REF_INSN (ref); + /* Make sure the next ref is for a different instruction, + so that we're not affected by the rescan. */ + next = DF_REF_NEXT_REG (ref); + while (next && DF_REF_INSN (next) == insn) + next = DF_REF_NEXT_REG (next); + + if (DEBUG_INSN_P (insn)) + { + bool changed = false; + for (; ref != next; ref = DF_REF_NEXT_REG (ref)) + { + rtx *loc = DF_REF_LOC (ref); + if (*loc == hard_frame_pointer_rtx) + { + *loc = plus_constant (Pmode, + stack_pointer_rtx, + -UNITS_PER_WORD); + changed = true; + } + } + if (changed) + df_insn_rescan (insn); + } + } + } + recompute_frame_layout_p = true; }