Re: [PATCH/RFC] Avoid invalid subregs during reload, call for testers

["Joseph S. Myers" <joseph at codesourcery dot com>]

On Mon, 4 Dec 2006, Rask Ingemann Lambertsen wrote:

>    This patch tries to address two cases where reload would call
> subreg_regno_offset() with invalid parameters. It also adds a gcc_assert()
> to catch new uses of subreg_regno_offset() with invalid parameters.

I think there are rather more cases where subreg_regno_offset may get 
called for an unrepresentable case.

PowerPC E500 double is very good at producing such cases.  You get such 
things as (subreg:DI (reg:DF)) and (subreg:DF (reg:DI)) to handle 
conversion between a double in a single 64-bit register (used for 
arithmetic) and in the low parts of two 32-bit registers (used for 
argument passing).  These subregs can't be converted directly to hard 
registers, but there are special instruction patterns (in spe.md) to 
handle moves to and from them.

It turns out there are two different sorts of callers of 
subreg_regno_offset and subreg_regno.  One sort requires the subreg to be 
representable as a hard reg; for this, the assert is correct.  The other 
sort doesn't, it only wishes to know what range of hard registers may be 
involved in a given subreg.  There are a lot of callers of the latter 
sort, computing a range of registers.

There are also bugs in the computation of that range of hard registers in 
some of those callers.  (subreg:DI (reg:DF)) (a DFmode register 
interpreted as DImode) takes 1 register, whereas a normal DImode value 
would take 2; (subreg:DF (reg:DI)) takes 2 registers, whereas a normal 
DFmode value would take 1.

My conclusion was that subreg_regno_offset and 
subreg_offset_representable_p should be combined into a single function 
that computes information about a subreg such as the offset, the number of 
hard registers and whether it is representable as a hard reg.  Those two 
functions would then become wrappers.

As a later stage, subreg_regno_offset would gain the assert of 
representability, with a different function added to provide the offset 
where representability isn't required; callers found not to need 
representability would be changed to use the new function.

The following is the part of my patch that creates the merged function and 
wrappers.  This is extracted from a larger patch implementing IBM long 
double support for E500 (which brings in many more of the problem subregs, 
such as (subreg:DI (reg:TF)) and (subreg:DF (reg:TI))).  That larger patch 
also changes the computations in various places of the number of registers 
in a subreg to use the information calculated more correctly here; but 
it's for a 4.1-based tree and the audit of hard_regno_nregs uses for ones 
that need to change would need repeating for current trunk.

Note that this patch has only been tested for E500 double, as part of a 
larger patch for the 4.1 tree.  But if something like this is thought 
useful for trunk as an initial step towards cleaning these things up, I 
can do some testing for mainline of this patch on its own (and also, 
though not immediately, prepare and test mainline versions of the other 
parts of the larger patch).

Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c	(revision 119396)
+++ gcc/rtlanal.c	(working copy)
@@ -38,6 +38,18 @@
 #include "function.h"
 #include "tree.h"
 
+/* Information about a subreg of a hard register.  */
+struct subreg_info
+{
+  /* Offset of first hard register involved in the subreg.  */
+  int offset;
+  /* Number of hard registers involved in the subreg.  */
+  int nregs;
+  /* Whether this subreg can be represented as a hard reg with the new
+     mode.  */
+  bool representable_p;
+};
+
 /* Forward declarations */
 static int global_reg_mentioned_p_1 (rtx *, void *);
 static void set_of_1 (rtx, rtx, void *);
@@ -46,6 +58,9 @@
 static int rtx_referenced_p_1 (rtx *, void *);
 static int computed_jump_p_1 (rtx);
 static void parms_set (rtx, rtx, void *);
+static void subreg_get_info (unsigned int, enum machine_mode,
+			     unsigned int, enum machine_mode,
+			     struct subreg_info *);
 
 static unsigned HOST_WIDE_INT cached_nonzero_bits (rtx, enum machine_mode,
                                                    rtx, enum machine_mode,
@@ -3177,69 +3194,27 @@
 		       SUBREG_BYTE (x));
 }
 
-/* This function returns the regno offset of a subreg expression.
+/* Fill in information about a subreg of a hard register.
    xregno - A regno of an inner hard subreg_reg (or what will become one).
    xmode  - The mode of xregno.
    offset - The byte offset.
    ymode  - The mode of a top level SUBREG (or what may become one).
-   RETURN - The regno offset which would be used.  */
-unsigned int
-subreg_regno_offset (unsigned int xregno, enum machine_mode xmode,
-		     unsigned int offset, enum machine_mode ymode)
+   info   - Pointer to structure to fill in.  */
+static void
+subreg_get_info (unsigned int xregno, enum machine_mode xmode,
+		 unsigned int offset, enum machine_mode ymode,
+		 struct subreg_info *info)
 {
   int nregs_xmode, nregs_ymode;
   int mode_multiple, nregs_multiple;
-  int y_offset;
+  int offset_adj, y_offset, y_offset_adj;
+  int regsize_xmode, regsize_ymode;
+  bool rknown;
 
   gcc_assert (xregno < FIRST_PSEUDO_REGISTER);
 
-  /* Adjust nregs_xmode to allow for 'holes'.  */
-  if (HARD_REGNO_NREGS_HAS_PADDING (xregno, xmode))
-    nregs_xmode = HARD_REGNO_NREGS_WITH_PADDING (xregno, xmode);
-  else
-    nregs_xmode = hard_regno_nregs[xregno][xmode];
-    
-  nregs_ymode = hard_regno_nregs[xregno][ymode];
+  rknown = false;
 
-  /* If this is a big endian paradoxical subreg, which uses more actual
-     hard registers than the original register, we must return a negative
-     offset so that we find the proper highpart of the register.  */
-  if (offset == 0
-      && nregs_ymode > nregs_xmode
-      && (GET_MODE_SIZE (ymode) > UNITS_PER_WORD
-	  ? WORDS_BIG_ENDIAN : BYTES_BIG_ENDIAN))
-    return nregs_xmode - nregs_ymode;
-
-  if (offset == 0 || nregs_xmode == nregs_ymode)
-    return 0;
-
-  /* Size of ymode must not be greater than the size of xmode.  */
-  mode_multiple = GET_MODE_SIZE (xmode) / GET_MODE_SIZE (ymode);
-  gcc_assert (mode_multiple != 0);
-
-  y_offset = offset / GET_MODE_SIZE (ymode);
-  nregs_multiple =  nregs_xmode / nregs_ymode;
-  return (y_offset / (mode_multiple / nregs_multiple)) * nregs_ymode;
-}
-
-/* This function returns true when the offset is representable via
-   subreg_offset in the given regno.
-   xregno - A regno of an inner hard subreg_reg (or what will become one).
-   xmode  - The mode of xregno.
-   offset - The byte offset.
-   ymode  - The mode of a top level SUBREG (or what may become one).
-   RETURN - Whether the offset is representable.  */
-bool
-subreg_offset_representable_p (unsigned int xregno, enum machine_mode xmode,
-			       unsigned int offset, enum machine_mode ymode)
-{
-  int nregs_xmode, nregs_ymode;
-  int mode_multiple, nregs_multiple;
-  int y_offset;
-  int regsize_xmode, regsize_ymode;
-
-  gcc_assert (xregno < FIRST_PSEUDO_REGISTER);
-
   /* If there are holes in a non-scalar mode in registers, we expect
      that it is made up of its units concatenated together.  */
   if (HARD_REGNO_NREGS_HAS_PADDING (xregno, xmode))
@@ -3272,7 +3247,10 @@
 	  && (offset / GET_MODE_SIZE (xmode_unit)
 	      != ((offset + GET_MODE_SIZE (ymode) - 1)
 		  / GET_MODE_SIZE (xmode_unit))))
-	return false;
+	{
+	  info->representable_p = false;
+	  rknown = true;
+	}
     }
   else
     nregs_xmode = hard_regno_nregs[xregno][xmode];
@@ -3280,24 +3258,53 @@
   nregs_ymode = hard_regno_nregs[xregno][ymode];
 
   /* Paradoxical subregs are otherwise valid.  */
-  if (offset == 0
-      && nregs_ymode > nregs_xmode
-      && (GET_MODE_SIZE (ymode) > UNITS_PER_WORD
-	  ? WORDS_BIG_ENDIAN : BYTES_BIG_ENDIAN))
-    return true;
+  if (!rknown
+      && offset == 0
+      && GET_MODE_SIZE (ymode) > GET_MODE_SIZE (xmode))
+    {
+      info->representable_p = true;
+      /* If this is a big endian paradoxical subreg, which uses more
+	 actual hard registers than the original register, we must
+	 return a negative offset so that we find the proper highpart
+	 of the register.  */
+      if (GET_MODE_SIZE (ymode) > UNITS_PER_WORD
+	  ? WORDS_BIG_ENDIAN : BYTES_BIG_ENDIAN)
+	info->offset = nregs_xmode - nregs_ymode;
+      else
+	info->offset = 0;
+      info->nregs = nregs_ymode;
+      return;
+    }
 
   /* If registers store different numbers of bits in the different
      modes, we cannot generally form this subreg.  */
   regsize_xmode = GET_MODE_SIZE (xmode) / nregs_xmode;
+  gcc_assert (regsize_xmode * nregs_xmode == GET_MODE_SIZE (xmode));
   regsize_ymode = GET_MODE_SIZE (ymode) / nregs_ymode;
-  if (regsize_xmode > regsize_ymode && nregs_ymode > 1)
-    return false;
-  if (regsize_ymode > regsize_xmode && nregs_xmode > 1)
-    return false;
+  gcc_assert (regsize_ymode * nregs_ymode == GET_MODE_SIZE (ymode));
+  if (!rknown && regsize_xmode > regsize_ymode && nregs_ymode > 1)
+    {
+      info->representable_p = false;
+      info->nregs
+	= (GET_MODE_SIZE (ymode) + regsize_xmode - 1) / regsize_xmode;
+      info->offset = offset / regsize_xmode;
+      return;
+    }
+  if (!rknown && regsize_ymode > regsize_xmode && nregs_xmode > 1)
+    {
+      info->representable_p = false;
+      info->nregs
+	= (GET_MODE_SIZE (ymode) + regsize_xmode - 1) / regsize_xmode;
+      info->offset = offset / regsize_xmode;
+      return;
+    }
 
   /* Lowpart subregs are otherwise valid.  */
-  if (offset == subreg_lowpart_offset (ymode, xmode))
-    return true;
+  if (!rknown && offset == subreg_lowpart_offset (ymode, xmode))
+    {
+      info->representable_p = true;
+      rknown = true;
+    }
 
   /* This should always pass, otherwise we don't know how to verify
      the constraint.  These conditions may be relaxed but
@@ -3308,24 +3315,63 @@
   /* The XMODE value can be seen as a vector of NREGS_XMODE
      values.  The subreg must represent a lowpart of given field.
      Compute what field it is.  */
-  offset -= subreg_lowpart_offset (ymode,
-				   mode_for_size (GET_MODE_BITSIZE (xmode)
-						  / nregs_xmode,
-						  MODE_INT, 0));
+  offset_adj = offset;
+  offset_adj -= subreg_lowpart_offset (ymode,
+				       mode_for_size (GET_MODE_BITSIZE (xmode)
+						      / nregs_xmode,
+						      MODE_INT, 0));
 
   /* Size of ymode must not be greater than the size of xmode.  */
   mode_multiple = GET_MODE_SIZE (xmode) / GET_MODE_SIZE (ymode);
   gcc_assert (mode_multiple != 0);
 
   y_offset = offset / GET_MODE_SIZE (ymode);
-  nregs_multiple =  nregs_xmode / nregs_ymode;
+  y_offset_adj = offset_adj / GET_MODE_SIZE (ymode);
+  nregs_multiple = nregs_xmode / nregs_ymode;
 
-  gcc_assert ((offset % GET_MODE_SIZE (ymode)) == 0);
+  gcc_assert ((offset_adj % GET_MODE_SIZE (ymode)) == 0);
   gcc_assert ((mode_multiple % nregs_multiple) == 0);
 
-  return (!(y_offset % (mode_multiple / nregs_multiple)));
+  if (!rknown)
+    {
+      info->representable_p = (!(y_offset_adj % (mode_multiple / nregs_multiple)));
+      rknown = true;
+    }
+  info->offset = (y_offset / (mode_multiple / nregs_multiple)) * nregs_ymode;
+  info->nregs = nregs_ymode;
 }
 
+/* This function returns the regno offset of a subreg expression.
+   xregno - A regno of an inner hard subreg_reg (or what will become one).
+   xmode  - The mode of xregno.
+   offset - The byte offset.
+   ymode  - The mode of a top level SUBREG (or what may become one).
+   RETURN - The regno offset which would be used.  */
+unsigned int
+subreg_regno_offset (unsigned int xregno, enum machine_mode xmode,
+		     unsigned int offset, enum machine_mode ymode)
+{
+  struct subreg_info info;
+  subreg_get_info (xregno, xmode, offset, ymode, &info);
+  return info.offset;
+}
+
+/* This function returns true when the offset is representable via
+   subreg_offset in the given regno.
+   xregno - A regno of an inner hard subreg_reg (or what will become one).
+   xmode  - The mode of xregno.
+   offset - The byte offset.
+   ymode  - The mode of a top level SUBREG (or what may become one).
+   RETURN - Whether the offset is representable.  */
+bool
+subreg_offset_representable_p (unsigned int xregno, enum machine_mode xmode,
+			       unsigned int offset, enum machine_mode ymode)
+{
+  struct subreg_info info;
+  subreg_get_info (xregno, xmode, offset, ymode, &info);
+  return info.representable_p;
+}
+
 /* Return the final regno that a subreg expression refers to.  */
 unsigned int
 subreg_regno (rtx x)

-- 
Joseph S. Myers
joseph@codesourcery.com