[PATCH] cprop_hardreg: Allow more propagation of the stack pointer.

[PATCH] cprop_hardreg: Allow more propagation of the stack pointer.

[Manolis Tsamis]

The stack pointer propagation fix 736f8fd3 turned out to be more restrictive
than needed by rejecting propagation of the stack pointer when REG_POINTER
didn't match.

This commit removes this check:
When the stack pointer is propagated it is fine for this to result in
REG_POINTER becoming true from false, which is what the original code checked.

This simplification makes the previously introduced function
maybe_copy_reg_attrs obsolete and the logic can be inlined at the call sites,
as it was before 736f8fd3.

gcc/ChangeLog:

	* regcprop.cc (maybe_copy_reg_attrs): Remove unnecessary function.
	(find_oldest_value_reg): Inline stack_pointer_rtx check.
	(copyprop_hardreg_forward_1): Inline stack_pointer_rtx check.

Signed-off-by: Manolis Tsamis <manolis.tsamis@vrull.eu>
---

 gcc/regcprop.cc | 56 ++++++++++++++++++++-----------------------------
 1 file changed, 23 insertions(+), 33 deletions(-)

diff --git a/gcc/regcprop.cc b/gcc/regcprop.cc
index d28a4d5aca8..a75af2ba7e3 100644
--- a/gcc/regcprop.cc
+++ b/gcc/regcprop.cc
@@ -451,31 +451,6 @@ maybe_mode_change (machine_mode orig_mode, machine_mode copy_mode,
   return NULL_RTX;
 }
 
-/* Helper function to copy attributes when replacing OLD_REG with NEW_REG.
-   If the changes required for NEW_REG are invalid return NULL_RTX, otherwise
-   return NEW_REG.  This is intended to be used with maybe_mode_change.  */
-
-static rtx
-maybe_copy_reg_attrs (rtx new_reg, rtx old_reg)
-{
-  if (new_reg != stack_pointer_rtx)
-    {
-      /* NEW_REG is assumed to be a register copy resulting from
-	 maybe_mode_change.  */
-      ORIGINAL_REGNO (new_reg) = ORIGINAL_REGNO (old_reg);
-      REG_ATTRS (new_reg) = REG_ATTRS (old_reg);
-      REG_POINTER (new_reg) = REG_POINTER (old_reg);
-    }
-  else if (REG_POINTER (new_reg) != REG_POINTER (old_reg))
-    {
-      /* Only a single instance of STACK_POINTER_RTX must exist and we cannot
-	 modify it.  Allow propagation if REG_POINTER for OLD_REG matches and
-	 don't touch ORIGINAL_REGNO and REG_ATTRS.  */
-      return NULL_RTX;
-    }
-  return new_reg;
-}
-
 /* Find the oldest copy of the value contained in REGNO that is in
    register class CL and has mode MODE.  If found, return an rtx
    of that oldest register, otherwise return NULL.  */
@@ -511,7 +486,17 @@ find_oldest_value_reg (enum reg_class cl, rtx reg, struct value_data *vd)
 
       new_rtx = maybe_mode_change (oldmode, vd->e[regno].mode, mode, i, regno);
       if (new_rtx)
-	return maybe_copy_reg_attrs (new_rtx, reg);
+	{
+	  /* NEW_RTX may be the global stack pointer rtx, in which case we
+	     must not modify it's attributes.  */
+	  if (new_rtx != stack_pointer_rtx)
+	    {
+	      ORIGINAL_REGNO (new_rtx) = ORIGINAL_REGNO (reg);
+	      REG_ATTRS (new_rtx) = REG_ATTRS (reg);
+	      REG_POINTER (new_rtx) = REG_POINTER (reg);
+	    }
+	  return new_rtx;
+	}
     }
 
   return NULL_RTX;
@@ -985,15 +970,20 @@ copyprop_hardreg_forward_1 (basic_block bb, struct value_data *vd)
 
 		  if (validate_change (insn, &SET_SRC (set), new_rtx, 0))
 		    {
-		      if (maybe_copy_reg_attrs (new_rtx, src))
+		      /* NEW_RTX may be the global stack pointer rtx, in which
+			 case we must not modify it's attributes.  */
+		      if (new_rtx != stack_pointer_rtx)
 			{
-			  if (dump_file)
-			    fprintf (dump_file,
-				     "insn %u: replaced reg %u with %u\n",
-				     INSN_UID (insn), regno, REGNO (new_rtx));
-			  changed = true;
-			  goto did_replacement;
+			  ORIGINAL_REGNO (new_rtx) = ORIGINAL_REGNO (src);
+			  REG_ATTRS (new_rtx) = REG_ATTRS (src);
+			  REG_POINTER (new_rtx) = REG_POINTER (src);
 			}
+		      if (dump_file)
+			fprintf (dump_file,
+				 "insn %u: replaced reg %u with %u\n",
+				 INSN_UID (insn), regno, REGNO (new_rtx));
+		      changed = true;
+		      goto did_replacement;
 		    }
 		  /* We need to re-extract as validate_change clobbers
 		     recog_data.  */
-- 
2.34.1

Re: [PATCH] cprop_hardreg: Allow more propagation of the stack pointer.

[Jeff Law]

On 8/7/23 05:31, Manolis Tsamis wrote:
> The stack pointer propagation fix 736f8fd3 turned out to be more restrictive
> than needed by rejecting propagation of the stack pointer when REG_POINTER
> didn't match.
> 
> This commit removes this check:
> When the stack pointer is propagated it is fine for this to result in
> REG_POINTER becoming true from false, which is what the original code checked.
> 
> This simplification makes the previously introduced function
> maybe_copy_reg_attrs obsolete and the logic can be inlined at the call sites,
> as it was before 736f8fd3.
> 
> gcc/ChangeLog:
> 
> 	* regcprop.cc (maybe_copy_reg_attrs): Remove unnecessary function.
> 	(find_oldest_value_reg): Inline stack_pointer_rtx check.
> 	(copyprop_hardreg_forward_1): Inline stack_pointer_rtx check.
OK
jeff

Re: [PATCH] cprop_hardreg: Allow more propagation of the stack pointer.

[Philipp Tomsich]

Applied to master, thanks!
--Philipp.


On Mon, 7 Aug 2023 at 19:20, Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 8/7/23 05:31, Manolis Tsamis wrote:
> > The stack pointer propagation fix 736f8fd3 turned out to be more restrictive
> > than needed by rejecting propagation of the stack pointer when REG_POINTER
> > didn't match.
> >
> > This commit removes this check:
> > When the stack pointer is propagated it is fine for this to result in
> > REG_POINTER becoming true from false, which is what the original code checked.
> >
> > This simplification makes the previously introduced function
> > maybe_copy_reg_attrs obsolete and the logic can be inlined at the call sites,
> > as it was before 736f8fd3.
> >
> > gcc/ChangeLog:
> >
> >       * regcprop.cc (maybe_copy_reg_attrs): Remove unnecessary function.
> >       (find_oldest_value_reg): Inline stack_pointer_rtx check.
> >       (copyprop_hardreg_forward_1): Inline stack_pointer_rtx check.
> OK
> jeff