Re: RFA: DSE replace_read revisited

[Richard Sandiford <rsandifo at nildram dot co dot uk>]

Eric Botcazou <ebotcazou@libertysurf.fr> writes:
>> Fair enough.  Since one of them has already objected, and no-one else
>> has spoken in favour of it, how about we simply limit the optimisation
>> to MODE_INT?
>
> Fine with me for now.

OK, I've bootstrapped & regression-tested the attached patch on
x86_64-linux-gnu.  I've also disabled the dse-1.c test for the
reasons stated in the comment.  OK to install?

Richard


gcc/
	* dse.c (find_shift_sequence): Always choose an integer mode for
	new_mode.
	(replace_read): Require both the read and store mode to be
	integer ones.  Remove a then-redundant FLOAT_P check.

gcc/testsuite/
	* gcc.target/mips/dse-1.c: Disable.

Index: gcc/dse.c
===================================================================
--- gcc/dse.c	(revision 129887)
+++ gcc/dse.c	(working copy)
@@ -1447,7 +1447,7 @@ find_shift_sequence (rtx read_reg,
 	continue;
 
       new_mode = smallest_mode_for_size (access_size * BITS_PER_UNIT,
-					 GET_MODE_CLASS (read_mode));
+					 MODE_INT);
       new_reg = gen_reg_rtx (new_mode);
 
       start_sequence ();
@@ -1473,9 +1473,8 @@ find_shift_sequence (rtx read_reg,
 	 of the arguments and could be precomputed.  It may
 	 not be worth doing so.  We could precompute if
 	 worthwhile or at least cache the results.  The result
-	 technically depends on SHIFT, ACCESS_SIZE, and
-	 GET_MODE_CLASS (READ_MODE).  But in practice the
-	 answer will depend only on ACCESS_SIZE.  */
+	 technically depends on both SHIFT and ACCESS_SIZE,
+	 but in practice the answer will depend only on ACCESS_SIZE.  */
 
       if (cost > COSTS_N_INSNS (1))
 	continue;
@@ -1557,7 +1556,8 @@ replace_read (store_info_t store_info, i
   if (!dbg_cnt (dse))
     return false;
 
-  if (GET_MODE_CLASS (read_mode) != GET_MODE_CLASS (store_mode))
+  if (GET_MODE_CLASS (read_mode) != MODE_INT
+      || GET_MODE_CLASS (store_mode) != MODE_INT)
     return false;
 
   /* To get here the read is within the boundaries of the write so
@@ -1580,7 +1580,7 @@ replace_read (store_info_t store_info, i
      call to get rid of the read.  */
   if (shift)
     {
-      if (access_size > UNITS_PER_WORD || FLOAT_MODE_P (store_mode))
+      if (access_size > UNITS_PER_WORD)
 	return false;
 
       shift_seq = find_shift_sequence (read_reg, access_size, store_info,
Index: gcc/testsuite/gcc.target/mips/dse-1.c
===================================================================
--- gcc/testsuite/gcc.target/mips/dse-1.c	(revision 129887)
+++ gcc/testsuite/gcc.target/mips/dse-1.c	(working copy)
@@ -1,3 +1,7 @@
+/* ??? Further to the subreg comment below, we can't rely on any of the
+   tests passing unless we handle subregs, and the patch to do so has
+   been rejected for the time being.  */
+/* { dg-do compile { target { ! *-*-* } } } */
 /* { dg-mips-options "-mgp64 -O" } */
 
 #define TEST(ID, TYPE1, TYPE2)					\