[patch] PR debug/53948

[patch] PR debug/53948

[Steven Bosscher]

[-- Attachment #1: Type: text/plain, Size: 1532 bytes --]

Hello,

This is my proposed fix for PR53948. We don't want to put user
variables in callee-clobbered registers, but obviously function
arguments are OK there. REG_USERVAR_P is set on PARM_DECLs and on user
variables, so it can't be used to distinguish between the two.

As it turns out, I can hi-jack a bit for that: 'unchanging' (currently
incorrectly documented as used on REG) for a new macro
REG_FUNCTION_PARM_P. I found one obvious place where this bit can be
used instead of REG_USERVAR_P, and probably there are a few more
places where this is useful (TBD, I'm going to look at all places
where RTL code looks at tree's PARM_DECL later).

Bootstrapped&tested on powerpc64-unknown-linux-gnu.
OK for trunk?

Ciao!
Steven

        PR debug/53948
        * rtl.h (REG_FUNCTION_PARM_P): New flag on a REG.  Re-use 'unchaning'.
        * emit-rtl.c (mark_function_parm_reg): New function.
        * function.c (assign_parm_setup_reg): Use mark_function_parm_reg
        instead of mark_user_reg.
        * combine.c (can_change_dest_mode): Preserve REG_FUNCTION_PARM_P.
        * web.c (entry_register): Likewise.
        * reload1.c (reload): Likewise.
        * ira-emit.c (ira_create_new_reg): Likewise.
        * reginfo.c (reg_scan_mark_refs): Likewise.
        * optabs.c (emit_libcall_block_1): Use REG_FUNCTION_PARM_P instead
        of REG_USERVAR_P.
        * regstat.c (dump_reg_info): Print REG_FUNCTION_PARM_P.
        * doc/rtl.texi (REG_FUNCTION_PARM_P): Document it.
        ('unchanging' flag): Fix documentation.

[-- Attachment #2: PR53948.diff --]
[-- Type: application/octet-stream, Size: 8972 bytes --]

	PR debug/53948
	* rtl.h (REG_FUNCTION_PARM_P): New flag on a REG.  Re-use 'unchanging'.
	* emit-rtl.c (mark_function_parm_reg): New function.
	* function.c (assign_parm_setup_reg): Use mark_function_parm_reg
	instead of mark_user_reg.
	* combine.c (can_change_dest_mode): Preserve REG_FUNCTION_PARM_P.
	* web.c (entry_register): Likewise.
	* reload1.c (reload): Likewise.
	* ira-emit.c (ira_create_new_reg): Likewise.
	* reginfo.c (reg_scan_mark_refs): Likewise.
	* optabs.c (emit_libcall_block_1): Use REG_FUNCTION_PARM_P instead
	of REG_USERVAR_P.
	* regstat.c (dump_reg_info): Print REG_FUNCTION_PARM_P.
	* doc/rtl.texi (REG_FUNCTION_PARM_P): Document it.
	('unchanging' flag): Fix documentation.

Index: rtl.h
===================================================================
--- rtl.h	(revision 189546)
+++ rtl.h	(working copy)
@@ -274,7 +274,8 @@ struct GTY((chain_next ("RTX_NEXT (&%h)"),
      1 in a CALL_INSN logically equivalent to
        ECF_LOOPING_CONST_OR_PURE and DECL_LOOPING_CONST_OR_PURE_P. */
   unsigned int call : 1;
-  /* 1 in a REG, MEM, or CONCAT if the value is set at most once, anywhere.
+  /* 1 in a REG if the register is a placeholder for a function parameter.
+     1 in a MEM, or CONCAT if the value is set at most once, anywhere.
      1 in a SUBREG used for SUBREG_PROMOTED_UNSIGNED_P.
      1 in a SYMBOL_REF if it addresses something in the per-function
      constants pool.
@@ -1101,6 +1102,10 @@ rhs_regno (const_rtx x)
 #define REG_USERVAR_P(RTX)						\
   (RTL_FLAG_CHECK1("REG_USERVAR_P", (RTX), REG)->volatil)
 
+/* 1 if RTX is a reg that corresponds to a function parameter.  */
+#define REG_FUNCTION_PARM_P(RTX)					\
+  (RTL_FLAG_CHECK1("REG_FUNCTION_PARM_P", (RTX), REG)->unchanging)
+
 /* 1 if RTX is a reg that holds a pointer value.  */
 #define REG_POINTER(RTX)						\
   (RTL_FLAG_CHECK1("REG_POINTER", (RTX), REG)->frame_related)
@@ -2415,6 +2420,7 @@ extern void maybe_set_first_label_num (rtx);
 extern void delete_insns_since (rtx);
 extern void mark_reg_pointer (rtx, int);
 extern void mark_user_reg (rtx);
+extern void mark_function_parm_reg (rtx);
 extern void reset_used_flags (rtx);
 extern void set_used_flags (rtx);
 extern void reorder_insns (rtx, rtx, rtx);
Index: emit-rtl.c
===================================================================
--- emit-rtl.c	(revision 189546)
+++ emit-rtl.c	(working copy)
@@ -1124,6 +1124,23 @@ mark_user_reg (rtx reg)
     }
 }
 
+/* Identify REG (which may be a CONCAT) as a user function parameter.  */
+
+void
+mark_function_parm_reg (rtx reg)
+{
+  if (GET_CODE (reg) == CONCAT)
+    {
+      REG_FUNCTION_PARM_P (XEXP (reg, 0)) = 1;
+      REG_FUNCTION_PARM_P (XEXP (reg, 1)) = 1;
+    }
+  else
+    {
+      gcc_assert (REG_P (reg));
+      REG_FUNCTION_PARM_P (reg) = 1;
+    }
+}
+
 /* Identify REG as a probable pointer register and show its alignment
    as ALIGN, if nonzero.  */
 
Index: function.c
===================================================================
--- function.c	(revision 189546)
+++ function.c	(working copy)
@@ -2923,7 +2923,7 @@ assign_parm_setup_reg (struct assign_parm_data_all
   parmreg = gen_reg_rtx (promoted_nominal_mode);
 
   if (!DECL_ARTIFICIAL (parm))
-    mark_user_reg (parmreg);
+    mark_function_parm_reg (parmreg);
 
   /* If this was an item that we received a pointer to,
      set DECL_RTL appropriately.  */
@@ -3075,7 +3075,7 @@ assign_parm_setup_reg (struct assign_parm_data_all
       /* We can't use nominal_mode, because it will have been set to
 	 Pmode above.  We must use the actual mode of the parm.  */
       parmreg = gen_reg_rtx (TYPE_MODE (TREE_TYPE (parm)));
-      mark_user_reg (parmreg);
+      mark_function_parm_reg (parmreg);
 
       if (GET_MODE (parmreg) != GET_MODE (DECL_RTL (parm)))
 	{
Index: combine.c
===================================================================
--- combine.c	(revision 189546)
+++ combine.c	(working copy)
@@ -2334,7 +2334,8 @@ can_change_dest_mode (rtx x, int added_sets, enum
 
   /* Or a pseudo that is only used once.  */
   return (REG_N_SETS (regno) == 1 && !added_sets
-	  && !REG_USERVAR_P (x));
+	  && !REG_USERVAR_P (x)
+	  && !REG_FUNCTION_PARM_P (x));
 }
 
 
Index: web.c
===================================================================
--- web.c	(revision 189546)
+++ web.c	(working copy)
@@ -263,6 +263,7 @@ entry_register (struct web_entry *entry, df_ref re
     {
       newreg = gen_reg_rtx (GET_MODE (reg));
       REG_USERVAR_P (newreg) = REG_USERVAR_P (reg);
+      REG_FUNCTION_PARM_P (newreg) = REG_FUNCTION_PARM_P (reg);
       REG_POINTER (newreg) = REG_POINTER (reg);
       REG_ATTRS (newreg) = REG_ATTRS (reg);
       if (dump_file)
Index: reload1.c
===================================================================
--- reload1.c	(revision 189546)
+++ reload1.c	(working copy)
@@ -1106,6 +1106,7 @@ reload (rtx first, int global)
 	      rtx reg = regno_reg_rtx[i];
 
 	      REG_USERVAR_P (reg) = 0;
+	      REG_FUNCTION_PARM_P (reg) = 0;
 	      PUT_CODE (reg, MEM);
 	      XEXP (reg, 0) = addr;
 	      if (reg_equiv_memory_loc (i))
Index: ira-emit.c
===================================================================
--- ira-emit.c	(revision 189546)
+++ ira-emit.c	(working copy)
@@ -335,6 +335,7 @@ ira_create_new_reg (rtx original_reg)
   new_reg = gen_reg_rtx (GET_MODE (original_reg));
   ORIGINAL_REGNO (new_reg) = ORIGINAL_REGNO (original_reg);
   REG_USERVAR_P (new_reg) = REG_USERVAR_P (original_reg);
+  REG_FUNCTION_PARM_P (new_reg) = REG_FUNCTION_PARM_P (original_reg);
   REG_POINTER (new_reg) = REG_POINTER (original_reg);
   REG_ATTRS (new_reg) = REG_ATTRS (original_reg);
   if (internal_flag_ira_verbose > 3 && ira_dump_file != NULL)
Index: reginfo.c
===================================================================
--- reginfo.c	(revision 189546)
+++ reginfo.c	(working copy)
@@ -1081,6 +1081,7 @@ reg_scan_mark_refs (rtx x, rtx insn)
 	     pseudo if this is the only set of that pseudo.  */
 	  && DF_REG_DEF_COUNT (REGNO (SET_DEST (x))) == 1
 	  && ! REG_USERVAR_P (SET_DEST (x))
+	  && ! REG_FUNCTION_PARM_P (SET_DEST (x))
 	  && ! REG_POINTER (SET_DEST (x))
 	  && ((REG_P (SET_SRC (x))
 	       && REG_POINTER (SET_SRC (x)))
Index: optabs.c
===================================================================
--- optabs.c	(revision 189546)
+++ optabs.c	(working copy)
@@ -3851,9 +3851,10 @@ emit_libcall_block_1 (rtx insns, rtx target, rtx r
   rtx final_dest = target;
   rtx next, last, insn;
 
-  /* If this is a reg with REG_USERVAR_P set, then it could possibly turn
-     into a MEM later.  Protect the libcall block from this change.  */
-  if (! REG_P (target) || REG_USERVAR_P (target))
+  /* If this is a reg with REG_FUNCTION_PARM_P set, then it could possibly turn
+     into a MEM later (e.g. if a REG_EQUIV note is attached to the insns that
+     sets the reg).  Protect the libcall block from this change.  */
+  if (! REG_P (target) || REG_FUNCTION_PARM_P (target))
     target = gen_reg_rtx (GET_MODE (target));
 
   /* If we're using non-call exceptions, a libcall corresponding to an
Index: regstat.c
===================================================================
--- regstat.c	(revision 189546)
+++ regstat.c	(working copy)
@@ -578,6 +578,8 @@ dump_reg_info (FILE *file)
 		 (DF_REG_DEF_COUNT (i) == 1) ? "" : "s");
       if (regno_reg_rtx[i] != NULL && REG_USERVAR_P (regno_reg_rtx[i]))
 	fputs ("; user var", file);
+      if (regno_reg_rtx[i] != NULL && REG_FUNCTION_PARM_P (regno_reg_rtx[i]))
+	fputs ("; function parm", file);
       if (REG_N_DEATHS (i) != 1)
 	fprintf (file, "; dies in %d places", REG_N_DEATHS (i));
       if (REG_N_CALLS_CROSSED (i) == 1)
Index: doc/rtl.texi
===================================================================
--- doc/rtl.texi	(revision 189546)
+++ doc/rtl.texi	(working copy)
@@ -731,6 +731,15 @@ The same hard register may be used also for collec
 functions called by this one, but @code{REG_FUNCTION_VALUE_P} is zero
 in this kind of use.
 
+@findex REG_FUNCTION_PARM_P
+@cindex @code{reg} and @samp{/u}
+@cindex @code{unchanging}, in @code{reg}
+@item REG_FUNCTION_PARM_P (@var{x})
+In a @code{reg}, nonzero if it corresponds to a function argument present
+in the user's source code.  Zero for arguments generated internally by
+the compiler.  Stored in the @code{unchanging} field and printed as
+@samp{/u}.  The flag uses its meaning after reload has completed.
+
 @findex RTX_FRAME_RELATED_P
 @cindex @code{insn} and @samp{/f}
 @cindex @code{call_insn} and @samp{/f}
@@ -978,7 +987,10 @@ In an RTL dump, this flag is represented as @samp{
 @findex unchanging
 @cindex @samp{/u} in RTL dump
 @item unchanging
-In @code{reg} and @code{mem} expressions, 1 means
+In @code{reg} expressions, 1 means
+that the register is used for a function argument.
+
+In @code{mem} expressions, 1 means
 that the value of the expression never changes.
 
 In @code{subreg} expressions, it is 1 if the @code{subreg} references an

Re: [patch] PR debug/53948

[Jan Kratochvil]

Hello Steven,

On Wed, 18 Jul 2012 19:46:16 +0200, Steven Bosscher wrote:
> This is my proposed fix for PR53948.

I can't speak for the GCC code but could it have a GCC testcase?


Thanks,
Jan

Re: [patch] PR debug/53948

[Steven Bosscher]

On Wed, Jul 18, 2012 at 7:55 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> Hello Steven,
>
> On Wed, 18 Jul 2012 19:46:16 +0200, Steven Bosscher wrote:
>> This is my proposed fix for PR53948.
>
> I can't speak for the GCC code but could it have a GCC testcase?

I wouldn't know what to test for. Looking for a .loc marker seems a bit fragile.

Ciao!
Steven

Re: [patch] PR debug/53948

[Jan Kratochvil]

On Wed, 18 Jul 2012 20:05:46 +0200, Steven Bosscher wrote:
> I wouldn't know what to test for. Looking for a .loc marker seems a bit fragile.

What is fragile on
// { dg-final { scan-assembler-times "\\.loc\t1 3 0\\r\\n\t\[^.\]" 6 } }

or something like that.  Line numbers are constant for the testcase.


Thanks,
Jan

Re: [patch] PR debug/53948

[Jakub Jelinek]

On Wed, Jul 18, 2012 at 08:55:17PM +0200, Jan Kratochvil wrote:
> On Wed, 18 Jul 2012 20:05:46 +0200, Steven Bosscher wrote:
> > I wouldn't know what to test for. Looking for a .loc marker seems a bit fragile.
> 
> What is fragile on
> // { dg-final { scan-assembler-times "\\.loc\t1 3 0\\r\\n\t\[^.\]" 6 } }
> 
> or something like that.  Line numbers are constant for the testcase.

Not all assemblers support .loc markers, sometimes .debug_line is emitted by
gcc directly, and not all targets use dwarf.  For the latter we can just put
the test into gcc.dg/debug/dwarf/, for the former we would need a new
dejagnu test.  But more importantly, on different arches I'd guess the
number of .loc times might be different.
Better than that would be a guality testcase limited to -O0.

	Jakub

Re: [patch] PR debug/53948

[Richard Biener]

On Wed, Jul 18, 2012 at 7:46 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> Hello,
>
> This is my proposed fix for PR53948. We don't want to put user
> variables in callee-clobbered registers, but obviously function
> arguments are OK there. REG_USERVAR_P is set on PARM_DECLs and on user
> variables, so it can't be used to distinguish between the two.
>
> As it turns out, I can hi-jack a bit for that: 'unchanging' (currently
> incorrectly documented as used on REG) for a new macro
> REG_FUNCTION_PARM_P. I found one obvious place where this bit can be
> used instead of REG_USERVAR_P, and probably there are a few more
> places where this is useful (TBD, I'm going to look at all places
> where RTL code looks at tree's PARM_DECL later).
>
> Bootstrapped&tested on powerpc64-unknown-linux-gnu.
> OK for trunk?

Just being pointed back to this patch ... I wonder if simply looking
at REG_EXPR of a REG_USERVAR_P reg and checking whether
it's a PARM_DECL isn't good enough (and simpler)?

I suppose the optabs.c change was the "real" fix, thus sth like

Index: gcc/optabs.c
===================================================================
--- gcc/optabs.c        (revision 194552)
+++ gcc/optabs.c        (working copy)
@@ -3848,9 +3848,13 @@ emit_libcall_block_1 (rtx insns, rtx tar
   rtx final_dest = target;
   rtx next, last, insn;

-  /* If this is a reg with REG_USERVAR_P set, then it could possibly turn
-     into a MEM later.  Protect the libcall block from this change.  */
-  if (! REG_P (target) || REG_USERVAR_P (target))
+  /* If this is a parameter with REG_USERVAR_P set, then it could possibly turn
+     into a MEM later (e.g. if a REG_EQUIV note is attached to the insns that
+     sets the reg).  Protect the libcall block from this change.  */
+  if (! REG_P (target)
+      || (REG_USERVAR_P (target)
+         && REG_EXPR (target) != NULL_TREE
+         && TREE_CODE (REG_EXPR (target)) == PARM_DECL))
     target = gen_reg_rtx (GET_MODE (target));

   /* If we're using non-call exceptions, a libcall corresponding to an

(untested)

Thanks,
Richard.

> Ciao!
> Steven
>
>         PR debug/53948
>         * rtl.h (REG_FUNCTION_PARM_P): New flag on a REG.  Re-use 'unchaning'.
>         * emit-rtl.c (mark_function_parm_reg): New function.
>         * function.c (assign_parm_setup_reg): Use mark_function_parm_reg
>         instead of mark_user_reg.
>         * combine.c (can_change_dest_mode): Preserve REG_FUNCTION_PARM_P.
>         * web.c (entry_register): Likewise.
>         * reload1.c (reload): Likewise.
>         * ira-emit.c (ira_create_new_reg): Likewise.
>         * reginfo.c (reg_scan_mark_refs): Likewise.
>         * optabs.c (emit_libcall_block_1): Use REG_FUNCTION_PARM_P instead
>         of REG_USERVAR_P.
>         * regstat.c (dump_reg_info): Print REG_FUNCTION_PARM_P.
>         * doc/rtl.texi (REG_FUNCTION_PARM_P): Document it.
>         ('unchanging' flag): Fix documentation.