Re: [patch] More thorough checking in reg_fits_class_p

[Jim MacArthur]

On 02/05/12 14:55, Richard Sandiford wrote:
Richard Earnshaw<rearn...@arm.com>  writes:
On 02/05/12 14:00, Richard Sandiford wrote:
Jim MacArthur<jim.macart...@arm.com>  writes:
New Changelog text:
2012-05-02 Jim MacArthur<jim.macart...@arm.com>
* recog.c (reg_fits_class_p): Check both regno and regno + offset are
hard registers.
Thanks.  I still think the final:

+       &&  HARD_REGISTER_NUM_P (end_hard_regno (regno + offset, mode))
check belongs in in_hard_reg_set_p, since most callers don't (and IMO
shouldn't need to) check this.  The idea behind adding these functions
was to commonise various bits of code that were doing the same checks
in slightly different ways.  Requiring each caller to check the end
register would go against that to some extent.

If you're going to do that (which is fine, BTW), I think
in_hard_reg_set_p should gcc_assert() that regno is a valid hard reg.
Sounds good.

Richard


Sorry for the delay in responding to this, I had a few problems with 
end_hard_regno. Here's a new version of the patch, which adds to 
in_hard_reg_set_p the assert and a check for the hardness of end_regno.  
end_hard_regno is the exclusive upper bound of the range, so not 
actually a meaningful reg no. HARD_REGNO_NREGS is required to return a 
positive value, so (end_regno - 1) is always safe, as I understand it.

I've tested this with an x86 bootstrap which shows no errors, and with 
our own AArch64 back end.

Jim

New ChangeLog text:

2012-05-17  Jim MacArthur<jim.macart...@arm.com>
      * recog.c (reg_fits_class_p): Check both regno and regno + offset are  
hard registers.
      * regs.h (in_hard_reg_set_p): Assert that regno is a hard register and
check end_regno - 1 is a hard register.

diff --git a/gcc/recog.c b/gcc/recog.c
index c5725d2..d664594 100644
--- a/gcc/recog.c
+++ b/gcc/recog.c
@@ -2792,14 +2792,16 @@ bool
 reg_fits_class_p (const_rtx operand, reg_class_t cl, int offset,
                  enum machine_mode mode)
 {
-  int regno = REGNO (operand);
+  unsigned int regno = REGNO (operand);
 
   if (cl == NO_REGS)
     return false;
 
+  /* Regno must not be a pseudo register.  Offset may be negative.  */
   return (HARD_REGISTER_NUM_P (regno)
-         && in_hard_reg_set_p (reg_class_contents[(int) cl],
-                               mode, regno + offset));
+         && HARD_REGISTER_NUM_P (regno + offset)
+         && in_hard_reg_set_p (reg_class_contents[(int) cl], mode, 
+                               regno + offset));
 }
 
 /* Split single instruction.  Helper function for split_all_insns and
diff --git a/gcc/regs.h b/gcc/regs.h
index 328b839..d18bf0a 100644
--- a/gcc/regs.h
+++ b/gcc/regs.h
@@ -24,6 +24,7 @@ along with GCC; see the file COPYING3.  If not see
 
 #include "machmode.h"
 #include "hard-reg-set.h"
+#include "rtl.h"
 
 #define REG_BYTES(R) mode_size[(int) GET_MODE (R)]
 
@@ -367,10 +368,16 @@ in_hard_reg_set_p (const HARD_REG_SET regs, enum 
machine_mode mode,
 {
   unsigned int end_regno;
 
+  gcc_assert (HARD_REGISTER_NUM_P (regno));
+  
   if (!TEST_HARD_REG_BIT (regs, regno))
     return false;
 
   end_regno = end_hard_regno (mode, regno);
+
+  if (!HARD_REGISTER_NUM_P (end_regno - 1))
+    return false;
+
   while (++regno < end_regno)
     if (!TEST_HARD_REG_BIT (regs, regno))
       return false;