PR19683: Fix a use of REG_CANNOT_CHANGE_MODE_P

[Richard Sandiford <rsandifo at redhat dot com>]

This patch fixes PR rtl-optimization/19683, which is caused by
a smallthinko in the reload inheritance code.  The situation is
that we're reloading:

        (subreg:SI (reg:DI PSEUDO) 4)

where PSEUDO is not allocated to a hard register.  The last register
used for reloading PSEUDO was the hard register pair (reg:DI R1)
and the backend does not allow R1 to change from DImode to SImode.

The inheritance code in choose_reload_regs looks like this:

             i = REGNO (last_reg);
  --A-->     i += subreg_regno_offset (i, GET_MODE (last_reg), byte, mode);
             last_class = REGNO_REG_CLASS (i);

             if (byte == 0)
               need_mode = mode;
             else
  --B-->       need_mode
  --B-->         = smallest_mode_for_size (GET_MODE_SIZE (mode) + byte,
  --B-->                                   GET_MODE_CLASS (mode));

             if (
          #ifdef CANNOT_CHANGE_MODE_CLASS
  --C-->         (!REG_CANNOT_CHANGE_MODE_P (i, GET_MODE (last_reg),
  --C-->                                     need_mode)
                  &&
          #endif
                 (GET_MODE_SIZE (GET_MODE (last_reg))
                  >= GET_MODE_SIZE (need_mode))
          #ifdef CANNOT_CHANGE_MODE_CLASS
                 )
          #endif
                 && ...

and for the situation above, we have:

        last_reg = (reg:DI R1)
        byte = 4
        mode = SImode

The first gotcha is the assignment to need_mode at (B).  It's trying to
compute a mode that is big enough to cover MODE plus BYTE leading bytes,
but the problem is that it's passing a byte count to smallest_mode_for_size,
and that function expects a bit count instead.  Thus we get:

        need_mode == smallest_mode_for_size (8, MODE_INT) == QImode

The mode passed in (C) therefore has no relation to the mode that
we're actually going to use.

Fixing (B) shows a further problem.  Once we have the expected:

    need_mode == smallest_mode_for_size (64, MODE_INT) == DImode

(C) will end up asking for:

    REG_CANNOT_CHANGE_MODE_P (i, DImode, DImode)

which is of course false.

NEED_MODE seems to be the wrong choice of third argument here.  Also
(and this is the crucial thing as far as the PR goes), "i" seems like
the wrong choice of register.  One use of CLASS_CANNOT_CHANGE_MODE is
to indicate cases where the endianness of a multi-register value is
different from the norm, so the computation of "i" at (A) might not
yield the right register.  In other words, one of the purposes of
this check is decide whether we can validly calculate "i" in the
first place.

Or, as an alternative argument: R_C_C_M_P (R, M1, M2) says whether we can
use mode M2 to refer to a value of mode M1 in register R.  That suggests
that R can actually hold a value of M1, which is not necessarily true
of "i".

I think the right check is:

    REG_CANNOT_CHANGE_MODE_P (REGNO (last_reg), GET_MODE (last_reg), mode)

since REGNO (last_reg) is the register that holds a value of mode
GET_MODE (last_reg) and MODE is the mode that we'll eventually use.

There's a possible tangential fix to the MIPS definition of
CANNOT_CHANGE_MODE_CLASS.  At the moment this doesn't handle mode
changes between word and multi-word values for the single-register
LO_REG class, on the basis that LO isn't allowed to store multi-word
values in the first place.  If the use of "i" in the check above is
thought to be valid, then we could change:

	  if (reg_classes_intersect_p (HI_REG, class))
	    return true;

to:

	  if (reg_classes_intersect_p (MD_REGS, class))
	    return true;

(I might well do that for 4.0 anyway, since it does seem more robust.
The logic behind the current code is a hang-over from the days where we
didn't have such fine-grained control.)

Bootstrapped & regression tested on mips64-linux-gnu and i686-pc-linux-gnu.
OK to install (4.0 and main)?  OK for 3.4 branch if the patch survives on
mainline for a week or so?

The need_mode problem has been around for a while but is a regression
from old releases.  More importantly, because the bug is sensitive to
register allocation, some testcases fail for 3.4 and pass with
mainline, and some testcases fail on mainline and pass with 3.4.
See the PR for examples.

Richard


	PR rtl-optimization/19683
	* reload1.c (choose_reload_regs): Pass the number of bits, not the
	number of bytes, to smallest_int_for_mode.  Fix arguments to
	REG_CANNOT_CHANGE_MODE_P.

testsuite/
	* gcc.dg/torture/pr19683-1.c: New test.

Index: reload1.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload1.c,v
retrieving revision 1.461
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -9 -r1.461 reload1.c
--- reload1.c	1 Feb 2005 07:22:14 -0000	1.461
+++ reload1.c	26 Feb 2005 09:32:29 -0000
@@ -5405,31 +5405,30 @@ choose_reload_regs (struct insn_chain *c
 
 		  i = REGNO (last_reg);
 		  i += subreg_regno_offset (i, GET_MODE (last_reg), byte, mode);
 		  last_class = REGNO_REG_CLASS (i);
 
 		  if (byte == 0)
 		    need_mode = mode;
 		  else
 		    need_mode
-		      = smallest_mode_for_size (GET_MODE_SIZE (mode) + byte,
+		      = smallest_mode_for_size (GET_MODE_BITSIZE (mode)
+						+ byte * BITS_PER_UNIT,
 						GET_MODE_CLASS (mode));
 
-		  if (
-#ifdef CANNOT_CHANGE_MODE_CLASS
-		      (!REG_CANNOT_CHANGE_MODE_P (i, GET_MODE (last_reg),
-						  need_mode)
-		       &&
-#endif
-		      (GET_MODE_SIZE (GET_MODE (last_reg))
+		  if ((GET_MODE_SIZE (GET_MODE (last_reg))
 		       >= GET_MODE_SIZE (need_mode))
 #ifdef CANNOT_CHANGE_MODE_CLASS
-		      )
+		      /* Verify that the register in "i" can be obtained
+			 from LAST_REG.  */
+		      && !REG_CANNOT_CHANGE_MODE_P (REGNO (last_reg),
+						    GET_MODE (last_reg),
+						    mode)
 #endif
 		      && reg_reloaded_contents[i] == regno
 		      && TEST_HARD_REG_BIT (reg_reloaded_valid, i)
 		      && HARD_REGNO_MODE_OK (i, rld[r].mode)
 		      && (TEST_HARD_REG_BIT (reg_class_contents[(int) class], i)
 			  /* Even if we can't use this register as a reload
 			     register, we might use it for reload_override_in,
 			     if copying it to the desired class is cheap
 			     enough.  */
diff -u /dev/null testsuite/gcc.dg/torture/pr19683-1.c
--- /dev/null	2005-01-29 16:17:39.000000000 +0000
+++ testsuite/gcc.dg/torture/pr19683-1.c	2005-02-26 08:13:00.000000000 +0000
@@ -0,0 +1,42 @@
+/* From PR rtl-optimization/19683.  On little-endian MIPS targets,
+   reload would incorrectly inherit the high part of the multiplication
+   result.  */
+/* { dg-do run { target mips*-*-* } } */
+
+extern void abort (void);
+extern void exit (int);
+
+#define REPEAT10(X, Y)					\
+  X(Y##0); X(Y##1); X(Y##2); X(Y##3); X(Y##4);		\
+  X(Y##5); X(Y##6); X(Y##7); X(Y##8); X(Y##9)
+
+#define REPEAT30(X) REPEAT10 (X, 0); REPEAT10 (X, 1); REPEAT10 (X, 2)
+#define IN(X) unsigned int x##X = ptr[0]
+#define OUT(X) ptr[0] = x##X
+
+union u { unsigned long long ll; unsigned int i[2]; };
+
+unsigned int
+foo (volatile unsigned int *ptr)
+{
+  union u u;
+  int result;
+
+  u.ll = (unsigned long long) ptr[0] * ptr[0];
+  REPEAT30 (IN);
+  REPEAT30 (OUT);
+  asm ("#" : "=l" (result) : "l" (u.i[1]));
+  return result;
+}
+
+int
+main (void)
+{
+  unsigned int array[] = { 1000 * 1000 * 1000 };
+  union u u;
+
+  u.ll = (unsigned long long) array[0] * array[0];
+  if (foo (array) != u.i[1])
+    abort ();
+  exit (0);
+}