validate_replace_rtx_1 fix version 3

Jan Hubicka jh@suse.cz
Thu Aug 3 12:46:00 GMT 2000


> On Sat, Jul 29, 2000 at 05:16:02PM +0200, Jan Hubicka wrote:
> > 
> > Hi
> > More experimenting with long long code found cut&paste typo in my patch.
> > 
> > Comment from previous mail:
> > The gcse replaces (subreg reg 0) by (subreg const_int 0) from time to time.
> > Problem is, that subreg with const_int lacks the inner mode and thus is
> > incorrect.  Gcse of course fails to match the operand, but constructs an
> > incorrect reg_equal note, that makes cse to blow up with my checking code.  The
> > subregs in questions are misinterpreted and may result in incorrect code so it
> > seems to be cleaner to me to avoid their existence by simplifying them in
> > validate_replace_rtx_1.  Beter code is produced too of course :)
> 
> This patch has one issue: IMHO the if (subreg_lowpart_p(x)) part should be
> killed, otherwise you get ICEs (gen_lowpart is not happy on stuff like 
> symbol_ref etc.).
> Attached below is a testcase which ICEs with this patch (ok to commit)?
OK. Solved by this patch.
The patch fixes couple of other problems present in the validate_replace_rtx
source - mainly the confusion of reg_equal and = testing and to simplify
nested subregs created by regmove.
Just running testsuite.

Thu Aug  3 21:44:38 MET DST 2000  Jan Hubicka  <jh@suse.cz>

	* recog.c (validate_replace_rtx_1): Fix confusion about equality
	testing; simplify subregs of constants and nested subregs.

Index: egcs/gcc/recog.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/recog.c,v
retrieving revision 1.71
diff -c -3 -p -r1.71 recog.c
*** recog.c	2000/07/31 08:42:26	1.71
--- recog.c	2000/08/03 19:44:19
*************** validate_replace_rtx_1 (loc, from, to, o
*** 459,471 ****
      case PLUS:
        /* If we have a PLUS whose second operand is now a CONST_INT, use
  	 plus_constant to try to simplify it.  */
!       if (GET_CODE (XEXP (x, 1)) == CONST_INT && XEXP (x, 1) == to)
! 	validate_change (object, loc, plus_constant (XEXP (x, 0), INTVAL (to)),
! 			 1);
        return;
  
      case MINUS:
!       if (GET_CODE (to) == CONST_INT && XEXP (x, 1) == from)
  	{
  	  validate_change (object, loc,
  			   plus_constant (XEXP (x, 0), - INTVAL (to)),
--- 459,472 ----
      case PLUS:
        /* If we have a PLUS whose second operand is now a CONST_INT, use
  	 plus_constant to try to simplify it.  */
!       if (GET_CODE (to) == CONST_INT && rtx_equal_p (XEXP (x, 1), to))
! 	{
! 	  validate_change (object, loc, plus_constant (XEXP (x, 0), INTVAL (to)),
! 			   1);
        return;
  
      case MINUS:
!       if (GET_CODE (to) == CONST_INT && rtx_equal_p (XEXP (x, 1), from))
  	{
  	  validate_change (object, loc,
  			   plus_constant (XEXP (x, 0), - INTVAL (to)),
*************** validate_replace_rtx_1 (loc, from, to, o
*** 482,491 ****
  	 in that case.  If it fails, substitute in something that we know
  	 won't be recognized.  */
        if (GET_MODE (to) == VOIDmode
! 	  && (XEXP (x, 0) == from
! 	      || (GET_CODE (XEXP (x, 0)) == REG && GET_CODE (from) == REG
! 		  && GET_MODE (XEXP (x, 0)) == GET_MODE (from)
! 		  && REGNO (XEXP (x, 0)) == REGNO (from))))
  	{
  	  rtx new = simplify_unary_operation (code, GET_MODE (x), to,
  					      GET_MODE (from));
--- 483,489 ----
  	 in that case.  If it fails, substitute in something that we know
  	 won't be recognized.  */
        if (GET_MODE (to) == VOIDmode
! 	  && rtx_equal_p (XEXP (x, 0), from))
  	{
  	  rtx new = simplify_unary_operation (code, GET_MODE (x), to,
  					      GET_MODE (from));
*************** validate_replace_rtx_1 (loc, from, to, o
*** 498,511 ****
        break;
  	
      case SUBREG:
        /* If we have a SUBREG of a register that we are replacing and we are
  	 replacing it with a MEM, make a new MEM and try replacing the
  	 SUBREG with it.  Don't do this if the MEM has a mode-dependent address
  	 or if we would be widening it.  */
  
!       if (SUBREG_REG (x) == from
! 	  && GET_CODE (from) == REG
  	  && GET_CODE (to) == MEM
  	  && ! mode_dependent_address_p (XEXP (to, 0))
  	  && ! MEM_VOLATILE_P (to)
  	  && GET_MODE_SIZE (GET_MODE (x)) <= GET_MODE_SIZE (GET_MODE (to)))
--- 496,564 ----
        break;
  	
      case SUBREG:
+       /* In case we are replacing by constant, attempt to simplify it to non-SUBREG
+          expression.  We can't do this later, since the information about inner mode
+          may be lost.  */
+       if (CONSTANT_P (to) && rtx_equal_p (SUBREG_REG (x), from))
+         {
+ 	  if (GET_MODE_SIZE (GET_MODE (x)) == UNITS_PER_WORD
+ 	      && GET_MODE_SIZE (GET_MODE (from)) > UNITS_PER_WORD
+ 	      && GET_MODE_CLASS (GET_MODE (x)) == MODE_INT)
+ 	    {
+ 	      rtx temp = operand_subword (to, SUBREG_WORD (x),
+ 					  0, GET_MODE (x));
+ 	      if (temp)
+ 		{
+ 		  validate_change (object, loc, temp, 1);
+ 		  return;
+ 		}
+ 	    }
+ 	  if (subreg_lowpart_p (x))
+ 	    {
+ 	      rtx new =  gen_lowpart_if_possible (GET_MODE (x), to);
+ 	      if (new)
+ 		{
+ 		  validate_change (object, loc, new, 1);
+ 		  return;
+ 		}
+ 	    }
+ 
+ 	  /* A paradoxical SUBREG of a VOIDmode constant is the same constant,
+ 	     since we are saying that the high bits don't matter.  */
+ 	  if (GET_MODE (to) == VOIDmode
+ 	      && GET_MODE_SIZE (GET_MODE (x)) > GET_MODE_SIZE (GET_MODE (from)))
+ 	    {
+ 	      validate_change (object, loc, to, 1);
+ 	      return;
+ 	    }
+         }
+ 
+       /* Changing mode twice with SUBREG => just change it once,
+ 	 or not at all if changing back to starting mode.  */
+       if (GET_CODE (to) == SUBREG
+ 	  && rtx_equal_p (SUBREG_REG (x), from))
+ 	{
+ 	  if (GET_MODE (x) == GET_MODE (SUBREG_REG (to))
+ 	      && SUBREG_WORD (x) == 0 && SUBREG_WORD (to) == 0)
+ 	    {
+ 	      validate_change (object, loc, SUBREG_REG (to), 1);
+ 	      return;
+ 	    }
+ 
+ 	  validate_change (object, loc,
+ 			   gen_rtx_SUBREG (GET_MODE (x), SUBREG_REG (to),
+ 					   SUBREG_WORD (x) + SUBREG_WORD (to)), 1);
+ 	  return;
+ 	}
+ 
        /* If we have a SUBREG of a register that we are replacing and we are
  	 replacing it with a MEM, make a new MEM and try replacing the
  	 SUBREG with it.  Don't do this if the MEM has a mode-dependent address
  	 or if we would be widening it.  */
  
!       if (GET_CODE (from) == REG
  	  && GET_CODE (to) == MEM
+ 	  && rtx_equal_p (SUBREG_REG (x), from)
  	  && ! mode_dependent_address_p (XEXP (to, 0))
  	  && ! MEM_VOLATILE_P (to)
  	  && GET_MODE_SIZE (GET_MODE (x)) <= GET_MODE_SIZE (GET_MODE (to)))
*************** validate_replace_rtx_1 (loc, from, to, o
*** 533,539 ****
  	 likely to be an insertion operation; if it was, nothing bad will
  	 happen, we might just fail in some cases).  */
  
!       if (XEXP (x, 0) == from && GET_CODE (from) == REG && GET_CODE (to) == MEM
  	  && GET_CODE (XEXP (x, 1)) == CONST_INT
  	  && GET_CODE (XEXP (x, 2)) == CONST_INT
  	  && ! mode_dependent_address_p (XEXP (to, 0))
--- 586,593 ----
  	 likely to be an insertion operation; if it was, nothing bad will
  	 happen, we might just fail in some cases).  */
  
!       if (GET_CODE (from) == REG && GET_CODE (to) == MEM
! 	  && rtx_equal_p (XEXP (x, 0), from)
  	  && GET_CODE (XEXP (x, 1)) == CONST_INT
  	  && GET_CODE (XEXP (x, 2)) == CONST_INT
  	  && ! mode_dependent_address_p (XEXP (to, 0))


More information about the Gcc-patches mailing list