i386: Replace frame pointer with stack pointer in debug insns

Message ID 20170812013228.GA3607@gmail.com
State New
Headers show

Commit Message

H.J. Lu Aug. 12, 2017, 1:32 a.m.
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(+)

Comments

Uros Bizjak Aug. 12, 2017, 8:28 a.m. | #1
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
>
Uros Bizjak Aug. 13, 2017, 4:15 p.m. | #2
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
>

Patch

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;
     }