[PATCH] combine: Count auto_inc properly (PR89794)

Segher Boessenkool segher@kernel.crashing.org
Sun Apr 14 09:56:00 GMT 2019


The code that checks if an auto-increment from i0 or i1 is not lost is
a bit shaky.  The code to check the same for i2 is non-existent, and
cannot be implemented in a similar way at all.  So, this patch counts
all auto-increments, and makes sure we end up with the same number as
we started with.  This works because we still have a check that we
will not duplicate any.

We should do this some better way, but not while we are in stage 4.

Tested on powerpc64-linux {-m32,-m64}; also tested manually on the Arm
testcase.

Comments?


Segher


2019-04-14  Segher Boessenkool  <segher@kernel.crashing.org>

	PR rtl-optimization/89794
	* combine.c (count_auto_inc): New function.
	(try_combine): Count how many auto_inc expressions there were in the
	original instructions.  Ensure we have the same number in the new
	instructions.  Remove the code that tried to ensure auto_inc side
	effects on i1 and i0 are not lost.

---
 gcc/combine.c | 60 +++++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 42 insertions(+), 18 deletions(-)

diff --git a/gcc/combine.c b/gcc/combine.c
index f681345..091adea 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -2667,6 +2667,16 @@ combine_remove_reg_equal_equiv_notes_for_regno (unsigned int regno)
     }
 }
 
+/* Callback function to count autoincs.  */
+
+int
+count_auto_inc (rtx, rtx, rtx, rtx, rtx, void *arg)
+{
+  (*((int *) arg))++;
+
+  return 0;
+}
+
 /* Try to combine the insns I0, I1 and I2 into I3.
    Here I0, I1 and I2 appear earlier than I3.
    I0 and I1 can be zero; then we combine just I2 into I3, or I1 and I2 into
@@ -2732,6 +2742,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
   int split_i2i3 = 0;
   int changed_i3_dest = 0;
   bool i2_was_move = false, i3_was_move = false;
+  int n_auto_inc = 0;
 
   int maxreg;
   rtx_insn *temp_insn;
@@ -3236,6 +3247,16 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
       return 0;
     }
 
+  /* Count how many auto_inc expressions there were in the original insns;
+     we need to have the same number in the resulting patterns.  */
+
+  if (i0)
+    for_each_inc_dec (PATTERN (i0), count_auto_inc, &n_auto_inc);
+  if (i1)
+    for_each_inc_dec (PATTERN (i1), count_auto_inc, &n_auto_inc);
+  for_each_inc_dec (PATTERN (i2), count_auto_inc, &n_auto_inc);
+  for_each_inc_dec (PATTERN (i3), count_auto_inc, &n_auto_inc);
+
   /* If the set in I2 needs to be kept around, we must make a copy of
      PATTERN (I2), so that when we substitute I1SRC for I1DEST in
      PATTERN (I2), we are only substituting for the original I1DEST, not into
@@ -3439,18 +3460,11 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
 
   if (i1 && GET_CODE (newpat) != CLOBBER)
     {
-      /* Check that an autoincrement side-effect on I1 has not been lost.
-	 This happens if I1DEST is mentioned in I2 and dies there, and
-	 has disappeared from the new pattern.  */
-      if ((FIND_REG_INC_NOTE (i1, NULL_RTX) != 0
-	   && i1_feeds_i2_n
-	   && dead_or_set_p (i2, i1dest)
-	   && !reg_overlap_mentioned_p (i1dest, newpat))
-	   /* Before we can do this substitution, we must redo the test done
-	      above (see detailed comments there) that ensures I1DEST isn't
-	      mentioned in any SETs in NEWPAT that are field assignments.  */
-	  || !combinable_i3pat (NULL, &newpat, i1dest, NULL_RTX, NULL_RTX,
-				0, 0, 0))
+      /* Before we can do this substitution, we must redo the test done
+	 above (see detailed comments there) that ensures I1DEST isn't
+	 mentioned in any SETs in NEWPAT that are field assignments.  */
+      if (!combinable_i3pat (NULL, &newpat, i1dest, NULL_RTX, NULL_RTX,
+			     0, 0, 0))
 	{
 	  undo_all ();
 	  return 0;
@@ -3480,12 +3494,8 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
 
   if (i0 && GET_CODE (newpat) != CLOBBER)
     {
-      if ((FIND_REG_INC_NOTE (i0, NULL_RTX) != 0
-	   && ((i0_feeds_i2_n && dead_or_set_p (i2, i0dest))
-	       || (i0_feeds_i1_n && dead_or_set_p (i1, i0dest)))
-	   && !reg_overlap_mentioned_p (i0dest, newpat))
-	  || !combinable_i3pat (NULL, &newpat, i0dest, NULL_RTX, NULL_RTX,
-				0, 0, 0))
+      if (!combinable_i3pat (NULL, &newpat, i0dest, NULL_RTX, NULL_RTX,
+			     0, 0, 0))
 	{
 	  undo_all ();
 	  return 0;
@@ -3506,6 +3516,20 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
       substed_i0 = 1;
     }
 
+  if (n_auto_inc)
+    {
+      int new_n_auto_inc = 0;
+      for_each_inc_dec (newpat, count_auto_inc, &new_n_auto_inc);
+
+      if (n_auto_inc != new_n_auto_inc)
+	{
+	  if (dump_file && (dump_flags & TDF_DETAILS))
+	    fprintf (dump_file, "Number of auto_inc expressions changed\n");
+	  undo_all ();
+	  return 0;
+	}
+    }
+
   /* Fail if an autoincrement side-effect has been duplicated.  Be careful
      to count all the ways that I2SRC and I1SRC can be used.  */
   if ((FIND_REG_INC_NOTE (i2, NULL_RTX) != 0
-- 
1.8.3.1



More information about the Gcc-patches mailing list