Re: Reload bug

[Eric Botcazou <ebotcazou@libertysurf.fr>]

> I am attaching full one.  It fixes the 3.2 testcases I have...
> Does it appear to make sense to you?

The locations from which you call the new subreg_offset_representable_p() 
look sensible. But I think there are some duplicate logic in 
reload_inner_reg_of_subreg() now.

+ /* This function returns true when the offset is representable via
+    subreg_offset in the given regno.

The function appears to assume that reg XREGNO supports mode YMODE. Is that 
by design? If so, I think you should mention it in the header comment.

+    RETURN - The regno offset which would be used.  */

Pasto.

+ bool
+ subreg_offset_representable_p (xregno, xmode, offset, ymode)
+      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;
+ 
+   if (xregno >= FIRST_PSEUDO_REGISTER)
+     abort ();
+ 
+   nregs_xmode = HARD_REGNO_NREGS (xregno, xmode);
+   nregs_ymode = HARD_REGNO_NREGS (xregno, ymode);
+ 
+   /* paradoxical subregs are always valid.  */
+   if (offset == 0
+       && nregs_ymode > nregs_xmode
+       && (GET_MODE_SIZE (ymode) > UNITS_PER_WORD
+ 	  ? WORDS_BIG_ENDIAN : BYTES_BIG_ENDIAN))
+     return true;
+ 
+   /* Lowpart subregs are always valid.  */
+   if (offset == subreg_lowpart_offset (ymode, xmode))
+     return true;

I think you should mention that the second "if" catches paradoxical subregs 
too.

+ #ifdef ENABLE_CHECKING
+   /* This should always pass, otherwise we don't know how to verify the
+      constraint. 
+ 
+      These conditions may be relaxed but subreg_offset would need to be
+      redesigned.  */
+   if (GET_MODE_SIZE (xmode) % GET_MODE_SIZE (ymode)
+       || GET_MODE_SIZE (ymode) % nregs_ymode
+       || mode_for_size (GET_MODE_SIZE (ymode) / nregs_ymode,
+ 	      		MODE_INT, 0) == VOIDmode
+       || nregs_xmode % nregs_ymode)
+     abort ();
+ #endif
+ 
+   /* The XMODE value can be seen as an vector of NREGS_XMODE
+      values.  The subreg must represent an lowpart of given field.
+      Compute what field it is.  */
+   offset -= subreg_lowpart_offset (mode_for_size (GET_MODE_SIZE (ymode)
+ 			  			  / nregs_ymode,
+ 						  MODE_INT, 0), xmode);

I'm clueless here: I think I understand what you want to do but I have a 
mixed feeling about the formula. In particular,

	GET_MODE_SIZE (ymode) / nregs_ymode

looks weird for a size. Didn't you forget to re-multiply by nregs_ymode?

+   /* size of ymode must not be greater than the size of xmode.  */
+   mode_multiple = GET_MODE_SIZE (xmode) / GET_MODE_SIZE (ymode);
+   if (mode_multiple == 0)
+     abort ();

All paradoxical subregs should already have been caught at that point, no?

+   y_offset = offset / GET_MODE_SIZE (ymode);
+   nregs_multiple =  nregs_xmode / nregs_ymode;
+ #ifdef ENABLE_CHECKING
+   if (offset % GET_MODE_SIZE (ymode)
+       || mode_multiple % nregs_multiple)
+     abort ();
+ #endif
+   return (!(y_offset % (mode_multiple / nregs_multiple)));
+ }

I think you're right about the condition.

-- 
Eric Botcazou