cprop_hardreg: Allow more propagation of the stack pointer.

Message ID 20230807113105.437693-1-manolis.tsamis@vrull.eu
State Unresolved
Headers
Series cprop_hardreg: Allow more propagation of the stack pointer. |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Manolis Tsamis Aug. 7, 2023, 11:31 a.m. UTC
  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(-)
  

Comments

Jeff Law Aug. 7, 2023, 5:20 p.m. UTC | #1
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
  
Philipp Tomsich Aug. 7, 2023, 5:32 p.m. UTC | #2
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
  

Patch

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.  */