PR target/21299 (reload accepting invalid asm)

Jan Hubicka jh@suse.cz
Tue Aug 8 13:51:00 GMT 2006


Hi,
the testcase in PR specify destination for DImode register as "ab" instead of
"A".  This hits difference in between reload and recog understanding of class
specifier - while recog is testing each letter separately and thus DImode
register won't fit to 'a' nor 'b' class, reload is always doing subunion of
alternatives before verifying, so it actually converts "ab" into "A" and is
satisfied.

This patch does two changes - one is obvious to make reg_fits_class_p called
for each specified class (this bring reload idea in sync with recog's and it
also will match more often in cases the specified constraints don't unify to
existing class).  This is the change introducing class variable.

Sadly this is not enough to fix the patch - the reload now correctly recognize
that insn needs reloads, but it still picks the first alternative and subunion
into "A" class for push_reload so it is later is satisfied with the instruction
again.

This don't seems easilly fixable to me and only thing I can see about it is to
exclude obviously stupid letters (at least for i386 I can't think of
counterexample where I can still trick reload to do mistake, other targets are
probably easier here, see comment in the patch).  The change walking all
pseudos is however ugly so I am open for all suggestions.

Bootstrapped/regtested i686-linux, OK?
:ADDPATCH middle-end:

2006-08-08  Jan Hubicka  <jh@suse.cz>
	PR target/21299
	* reload.c (find_reloads): Ignore class specifiers for classes that
	can't contain given mode; check each specifier separately instead of
	union to see if alternative match.
Index: reload.c
===================================================================
-u -L reload.c	(revision 115995) -L reload.c	(working copy) .svn/text-base/reload.c.svn-base reload.c
--- reload.c	(revision 115995)
+++ reload.c	(working copy)
@@ -2913,6 +2913,7 @@ find_reloads (rtx insn, int replace, int
 	     operand.  */
 	  int constmemok = 0;
 	  int earlyclobber = 0;
+	  enum reg_class class = NO_REGS;
 
 	  /* If the predicate accepts a unary operator, it means that
 	     we need to reload the operand, but do not do this for
@@ -3309,6 +3310,7 @@ find_reloads (rtx insn, int replace, int
 		/* Drop through into 'r' case.  */
 
 	      case 'r':
+		class = GENERAL_REGS;
 		this_alternative[i]
 		  = (int) reg_class_subunion[this_alternative[i]][(int) GENERAL_REGS];
 		goto reg;
@@ -3368,17 +3370,40 @@ find_reloads (rtx insn, int replace, int
 		    break;
 		  }
 
+		class = REG_CLASS_FROM_CONSTRAINT (c, p);
+		/* When class is too tiny to hold the operand, ignore it.
+		   See PR21299.  First quickly test if things looks slopy
+		   and if they does try to look if register is really
+		   available.  */
+		if (CLASS_MAX_NREGS (class, GET_MODE (recog_data.operand[i]))
+		    > (int)reg_class_size [class])
+		  {
+		    int regno;
+		    enum machine_mode mode = GET_MODE (recog_data.operand[i]);
+		    for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
+		      if (TEST_HARD_REG_BIT (reg_class_contents[class], regno)
+			  && HARD_REGNO_MODE_OK (regno, mode)
+			  && (regno + hard_regno_nregs[regno][mode]
+			      < FIRST_PSEUDO_REGISTER)
+			  && TEST_HARD_REG_BIT
+				 (reg_class_contents[class],
+				  regno + hard_regno_nregs[regno][mode]))
+		     break;
+		   if (regno == FIRST_PSEUDO_REGISTER)
+		     break;
+		  }
 		this_alternative[i]
 		  = (int) (reg_class_subunion
 			   [this_alternative[i]]
-			   [(int) REG_CLASS_FROM_CONSTRAINT (c, p)]);
+			   [(int) class]);
 	      reg:
 		if (GET_MODE (operand) == BLKmode)
 		  break;
 		winreg = 1;
 		if (REG_P (operand)
-		    && reg_fits_class_p (operand, this_alternative[i],
-					 offset, GET_MODE (recog_data.operand[i])))
+		    && reg_fits_class_p (operand, class,
+					 offset,
+					 GET_MODE (recog_data.operand[i])))
 		  win = 1;
 		break;
 	      }



More information about the Gcc-patches mailing list