This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA] Fix combine to canonicalize (mult X pow2)) more often



When combine needs to split a complex insn, it will canonicalize a simple (mult X (const_int Y)) where Y is a power of 2 into the expected (ashift X (const_int Y')) if the (mult ...) is selected as a split point.

However if the split point is (plus (mult (X (const_int Y)) Z) combine fails to canonicalize.

In this particular testcase we end up with two expressions which produce the same value, one is in canonical form using ASHIFT, the other in non-canonical form with a MULT. Because the two forms differ, we fail to find the common subexpression in postreload-gcse.

Bootstrapped and regression tested on x86_64-unknown-linux-gnu. Also successfully ran hppa.exp with hppa2.0w-hp-hpux11 cross compiler. With this change we generate the same code for my 300+ files on the PA that we had prior to Venkat's combine changes.

The remaining patches in this series will just be cleaning up the PA backend, in particular removing all cases where we might generate non-canonical forms. You could legitimately ask if that would eliminate the need for this change. It will not because the MULT form is still canonical inside a MEM and we might need to split out an address calculation from the MEM to make an insn recognizable.

OK for the trunk?

Jeff

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index f4012b7..425813c 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,8 @@
 2015-05-21  Jeff Law  <law@redhat.com>
 
+	* combine.c (try_combine): Canonicalize (plus (mult X pow2) Y) into
+	(plus (ashift X log2) Y) if it is a split point.
+
 	* combine.c (find_split_point): Handle ASHIFT like MULT to encourage
 	multiply-accumulate/shift-add insn generation.
 
diff --git a/gcc/combine.c b/gcc/combine.c
index 8c527a7..2cb9fd2 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -3749,6 +3749,21 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
 	      split_code = GET_CODE (*split);
 	    }
 
+	  /* Similarly for (plus (mult FOO (const_int pow2))).  */
+	  if (split_code == PLUS
+	      && GET_CODE (XEXP (*split, 0)) == MULT
+	      && CONST_INT_P (XEXP (XEXP (*split, 0), 1))
+	      && INTVAL (XEXP (XEXP (*split, 0), 1)) > 0
+	      && (i = exact_log2 (UINTVAL (XEXP (XEXP (*split, 0), 1)))) >= 0)
+	    {
+	      rtx nsplit = XEXP (*split, 0);
+	      SUBST (XEXP (*split, 0), gen_rtx_ASHIFT (GET_MODE (nsplit),
+					     XEXP (nsplit, 0), GEN_INT (i)));
+	      /* Update split_code because we may not have a multiply
+		 anymore.  */
+	      split_code = GET_CODE (*split);
+	    }
+
 #ifdef INSN_SCHEDULING
 	  /* If *SPLIT is a paradoxical SUBREG, when we split it, it should
 	     be written as a ZERO_EXTEND.  */
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 283644c..41a09bad 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,5 +1,6 @@
 2015-05-21  Jeff Law  <law@redhat.com>
 
+	* gcc.target/hppa/shadd-3.c: New test.
 	* gcc.target/hppa/shadd-2.c: New test.
 
 2015-05-21  Oleg Endo  <olegendo@gcc.gnu.org>
diff --git a/gcc/testsuite/gcc.target/hppa/shadd-3.c b/gcc/testsuite/gcc.target/hppa/shadd-3.c
new file mode 100644
index 0000000..f0443ea
--- /dev/null
+++ b/gcc/testsuite/gcc.target/hppa/shadd-3.c
@@ -0,0 +1,41 @@
+/* { dg-do compile }  */
+/* { dg-options "-O2" }  */
+/* In this test we want to verify that combine canonicalizes the
+   MULT into an ASHIFT which in turn allows postreload-gcse to
+   find the common subexpression.
+
+   Neither pass dumps stuff in a format that is particularly good
+   for parsing here, so we count the shadd insns.  More is not
+   necessarily better in this test.  If this test is too fragile
+   over time we'll have to revisit the combine and/or postreload
+   dumps.  */
+/* { dg-final { scan-assembler-times "sh.add" 5 } }  */
+
+extern void oof (void);
+typedef struct simple_bitmap_def *sbitmap;
+struct simple_bitmap_def
+{
+  unsigned char *popcount;
+  unsigned int n_bits;
+  unsigned long elms[1];
+};
+__inline__ void
+SET_BIT (sbitmap map, unsigned int bitno)
+{
+  if (map->popcount)
+    {
+      unsigned char oldbit;
+      oldbit =
+	((map)->elms[bitno / 64]);
+      if (!oldbit)
+	oof ();
+    }
+  map->elms[bitno / 64] |= 1;
+}
+
+void
+fix_bb_placements (int indx1, int indx2, sbitmap in_queue)
+{
+  SET_BIT (in_queue, indx1);
+  SET_BIT (in_queue, indx2);
+}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]