This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] More thorough checking in reg_fits_class_p

Richard Earnshaw schrieb:
On 30/04/12 15:07, Richard Sandiford wrote:

Richard Earnshaw writes:

Jim MacArthur wrote:

The current code in reg_fits_class_p appears to be incorrect; since offset may be negative, it's necessary to check both ends of the range otherwise an array overrun or underrun may occur when calling in_hard_reg_set_p. in_hard_reg_set_p should also be checked for each register in the range of regno .. regno+offset.

A negative offset can occur on a big-endian target. For example, on AArch64, subreg_regno_offset (0, DImode, 0, TImode) returns -1.

We discovered this problem while developing unrelated code for big-endian support in the AArch64 back end.

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

Ok for trunk?

gcc/Changelog entry:

2012-04-26 Jim MacArthur<>
     * recog.c (reg_fits_class_p): Check every register between regno and
       regno+offset is in the hard register set.




diff --git a/gcc/recog.c b/gcc/recog.c
index 8fb96a0..825bfb1 100644
--- a/gcc/recog.c
+++ b/gcc/recog.c
@@ -2759,14 +2759,28 @@ 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;

-  return (HARD_REGISTER_NUM_P (regno)
-	  && in_hard_reg_set_p (reg_class_contents[(int) cl],
-				mode, regno + offset));
+  /* We should not assume all registers in the range regno to regno + offset are
+     valid just because each end of the range is.  */
+  if (HARD_REGISTER_NUM_P (regno) && HARD_REGISTER_NUM_P (regno + offset))
+    {
+      unsigned int i;
+      unsigned int start = MIN (regno, regno + offset);
+      unsigned int end = MAX (regno, regno + offset);
+      for (i = start; i <= end; i++)
+	{
+	  if (!in_hard_reg_set_p (reg_class_contents[(int) cl],
+				  mode, i))
+	    return false;
+	}

This doesn't look right to me. We should still only need to check in_hard_reg_set_p for one register number. I'd have expected something like:

 return (HARD_REGISTER_NUM_P (regno)
	  && HARD_REGISTER_NUM_P (regno + offset)
	  && in_hard_reg_set_p (reg_class_contents[(int) cl],
				mode, regno + offset));



Hmm, looking at the comment that precedes the function makes me think the actual implementation should be:

	int regno = REGNO (operand) + offset;

	return (HARD_REGISTER_NUM_P (regno)
		&& HARD_REGISTER_NUM_P (end_hard_regno (regno, mode))
		&& in_hard_reg_set_p (...., regno);


That is, the original regno isn't really interesting and what really
counts is the range regno + offset ... regno + offset +
NUM_HARD_REGS(mode) - 1

Shouldn't this be HARD_REGNO_NREGS?


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]