Improve sanity checking for erroneous insns an' da "g" thang

Hans-Peter Nilsson hans-peter.nilsson@axis.com
Sat Nov 12 02:18:00 GMT 2005


See PR 24750, where reload created an invalid insn that wasn't
diagnosed until target operand output time.

Accepting non-general_operands isn't the intention according to
the comment and isn't supported by the documentation (assuming
that "any register, memory or immediate integer operand is
allowed, except for registers that are not general registers"
doesn't imply that invalid operands are supposed to be matched).

The problem with relying on recog () (code in insn-recog.c) is
that it's not called by reload, so bugs in reload aren't caught
in sanity-checks, unless later RTL passes happen to re-recog the
insn for some reason.

It confused me once before, when writing PIC support for CRIS.
See "*movsi_internal" in cris.md, which *used* to have a
predicate matching general_operand-and-PIC-decorations
(r101811), where I had to match the constraint for the
PIC-decorations (being non-general_operands) *before* "g",
because "g" matched anyway.  (The note there used to be a FIXME
speaking of this bug, but now the PIC-decorations are supposed
to be general_operands anyway, so it's just a caveat.)

This *will* cause different code, if a "g" is not the last
otherwise-matching non-register constraint but that *shouldn't*
happen with correct constraints.  It will cause invalid insns
to be spotted at the first constrain_operands (>=0) call,
possibly opening up for spotting currently dormant bugs.  For
example, for gcc.target/cris/torture/pr24750-2.c without the
correction, we'd see the expected:

/tmp/pr24750-2.c:20: error: insn does not satisfy its constraints:
(insn 23 35 29 0 (set (reg/i:SI 10 r10 [ <result> ])
        (sign_extend:SI (mem:QI (plus:SI (sign_extend:SI (reg:HI 8 r8))
                    (reg:SI 9 r9)) [0 S1 A8]))) 51 {extendqisi2}
(insn_list:REG_DEP_TRUE 6 (insn_list:REG_DEP_TRUE 7 (nil)))
    (nil))
/tmp/pr24750-2.c:20: internal compiler error: in reload_cse_simplify_operands, at postreload.c:393

Tested native on x86_64-unknown-linux-gnu and cross to
mmix-knuth-mmixware, cris-axis-linux-gnu and cris-axis-elf.

Ok to commit?  Perhaps wait for 4.2?

	* recog.c (constrain_operands) <case 'g'>: For a match, require
	that a non-register matches general_operand when strict >= 0.

Index: recog.c
===================================================================
--- recog.c	(revision 106481)
+++ recog.c	(working copy)
@@ -2429,16 +2429,22 @@ constrain_operands (int strict)
 		break;
 
 		/* No need to check general_operand again;
-		   it was done in insn-recog.c.  */
+		   it was done in insn-recog.c.  Well, except that reload
+		   doesn't check the validity of its replacements, but
+		   that should only matter when there's a bug.  */
 	      case 'g':
 		/* Anything goes unless it is a REG and really has a hard reg
 		   but the hard reg is not in the class GENERAL_REGS.  */
-		if (strict < 0
-		    || GENERAL_REGS == ALL_REGS
-		    || !REG_P (op)
-		    || (reload_in_progress
-			&& REGNO (op) >= FIRST_PSEUDO_REGISTER)
-		    || reg_fits_class_p (op, GENERAL_REGS, offset, mode))
+		if (REG_P (op))
+		  {
+		    if (strict < 0
+			|| GENERAL_REGS == ALL_REGS
+			|| (reload_in_progress
+			    && REGNO (op) >= FIRST_PSEUDO_REGISTER)
+			|| reg_fits_class_p (op, GENERAL_REGS, offset, mode))
+		      win = 1;
+		  }
+		else if (strict < 0 || general_operand (op, mode))
 		  win = 1;
 		break;
 

brgds, H-P



More information about the Gcc-patches mailing list