This is the mail archive of the gcc-patches@gcc.gnu.org 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]

[committed] Fix register elimination of auto-inc expressions


Hello,

this patch cleans up register elimination handling for auto-inc expressions,
and fixes some latent bugs (that are exposed on the dataflow branch).

An auto-inc expression has two subexpressions: the register that is being
implicitly incremented, and the increment amount.  The latter can be 
implicit (PRE_INC/PRE_DEC/POST_INC/POST_DEC) or explicit (PRE_MODIFY/
POST_MODIFY), and in the latter case either constant or a register.

Register elimination occurs in two flavours: a source register can be
replaced by a target register plus some (constant) offset according to
an elimination rule, or an invariant register can be replaced by some
stack address (stack pointer plus offset).

As a register modified by an auto-inc expression cannot be invariant,
we thus have the following cases to consider:

1. The incremented register occurs as source of an elimination rule.

2. The incremented register occurs as target of an elimination rule.

3. The increment amount register is invariant.

4. The increment amount register occurs as source of an elimination rule.

5. The increment amount register occurs as target of an elimination rule.
   (This has no effect on register elimiation.)


If case 1. occurs, we simply disable the elimination rule in question.
This is in line with the general strategy to not allow elimination of
registers that are modified.  See also the discussion at:
http://gcc.gnu.org/ml/gcc-patches/2006-07/msg00152.html

If case 2. occurs, we need to make sure the increment amount is constant
so that we can adjust the elimination offset accordingly.  If the amount
is non-constant, we again need to disable the elimination rule.

For cases 3. and 4. we allow the elimination to proceed.  This leads to
a PLUS subexpression temporarily inserted as increment amount, which is
later cleaned up by find_reloads_address_1.


The following patch implements those ground rules.  It also removes an
(unreachable) block of code attempting to handle a MEM subexpression of
a PRE_INC/PRE_DEC/POST_INC/POST_DEC -- this can never happen (today).


Bootstrapped/regtested on s390-ibm-linux, s390x-ibm-linux and
powerpc64-ibm-linux.  Also bootstrapped/regtested on the dataflow branch
on powerpc64-ibm-linux, after reverting the reload1.c change from:
http://gcc.gnu.org/ml/gcc-patches/2006-07/msg00037.html
(Fixes an -m64 bug there.)

Committed to mainline.  Also committed to the dataflow branch (after
reverting the above reload1.c change).

Bye,
Ulrich


ChangeLog:

	* reload.c (find_reloads_address_1): Handle PLUS expressions resulting
	from register elimination as PRE_MODIFY / POST_MODIFY increments.
	Do not attempt to handle MEM inside auto-inc expressions.
	* reload1.c (eliminate_regs_1): Do not attempt to handle elimination
	of a register modified by an auto-inc expression.  However, do handle
	elimination of a register used as PRE_MODIFY / POST_MODIFY increment.
	(elimination_effects): Prohibit elimination of a register modified
	by an auto-inc expression.  Disable register elimination rules whose
	target register is modified by an auto-inc expression with variable
	increment.


Index: gcc/reload.c
===================================================================
*** gcc/reload.c	(revision 122132)
--- gcc/reload.c	(working copy)
*************** find_reloads_address_1 (enum machine_mod
*** 5543,5558 ****
  	   auto-modify by a constant then we could try replacing a pseudo
  	   register with its equivalent constant where applicable.
  
  	   If we later decide to reload the whole PRE_MODIFY or
  	   POST_MODIFY, inc_for_reload might clobber the reload register
  	   before reading the index.  The index register might therefore
  	   need to live longer than a TYPE reload normally would, so be
  	   conservative and class it as RELOAD_OTHER.  */
! 	if (REG_P (XEXP (op1, 1)))
! 	  if (!REGNO_OK_FOR_INDEX_P (REGNO (XEXP (op1, 1))))
! 	    find_reloads_address_1 (mode, XEXP (op1, 1), 1, code, SCRATCH,
! 				    &XEXP (op1, 1), opnum, RELOAD_OTHER,
! 				    ind_levels, insn);
  
  	gcc_assert (REG_P (XEXP (op1, 0)));
  
--- 5543,5562 ----
  	   auto-modify by a constant then we could try replacing a pseudo
  	   register with its equivalent constant where applicable.
  
+ 	   We also handle the case where the register was eliminated
+ 	   resulting in a PLUS subexpression.
+ 
  	   If we later decide to reload the whole PRE_MODIFY or
  	   POST_MODIFY, inc_for_reload might clobber the reload register
  	   before reading the index.  The index register might therefore
  	   need to live longer than a TYPE reload normally would, so be
  	   conservative and class it as RELOAD_OTHER.  */
! 	if ((REG_P (XEXP (op1, 1))
! 	     && !REGNO_OK_FOR_INDEX_P (REGNO (XEXP (op1, 1))))
! 	    || GET_CODE (XEXP (op1, 1)) == PLUS)
! 	  find_reloads_address_1 (mode, XEXP (op1, 1), 1, code, SCRATCH,
! 				  &XEXP (op1, 1), opnum, RELOAD_OTHER,
! 				  ind_levels, insn);
  
  	gcc_assert (REG_P (XEXP (op1, 0)));
  
*************** find_reloads_address_1 (enum machine_mod
*** 5733,5775 ****
  	    }
  	  return value;
  	}
- 
-       else if (MEM_P (XEXP (x, 0)))
- 	{
- 	  /* This is probably the result of a substitution, by eliminate_regs,
- 	     of an equivalent address for a pseudo that was not allocated to a
- 	     hard register.  Verify that the specified address is valid and
- 	     reload it into a register.  */
- 	  /* Variable `tem' might or might not be used in FIND_REG_INC_NOTE.  */
- 	  rtx tem ATTRIBUTE_UNUSED = XEXP (x, 0);
- 	  rtx link;
- 	  int reloadnum;
- 
- 	  /* Since we know we are going to reload this item, don't decrement
- 	     for the indirection level.
- 
- 	     Note that this is actually conservative:  it would be slightly
- 	     more efficient to use the value of SPILL_INDIRECT_LEVELS from
- 	     reload1.c here.  */
- 	  /* We can't use ADDR_TYPE (type) here, because we need to
- 	     write back the value after reading it, hence we actually
- 	     need two registers.  */
- 	  find_reloads_address (GET_MODE (x), &XEXP (x, 0),
- 				XEXP (XEXP (x, 0), 0), &XEXP (XEXP (x, 0), 0),
- 				opnum, type, ind_levels, insn);
- 
- 	  reloadnum = push_reload (x, NULL_RTX, loc, (rtx*) 0,
- 				   context_reg_class,
- 				   GET_MODE (x), VOIDmode, 0, 0, opnum, type);
- 	  rld[reloadnum].inc
- 	    = find_inc_amount (PATTERN (this_insn), XEXP (x, 0));
- 
- 	  link = FIND_REG_INC_NOTE (this_insn, tem);
- 	  if (link != 0)
- 	    push_replacement (&XEXP (link, 0), reloadnum, VOIDmode);
- 
- 	  return 1;
- 	}
        return 0;
  
      case TRUNCATE:
--- 5737,5742 ----
Index: gcc/reload1.c
===================================================================
*** gcc/reload1.c	(revision 122199)
--- gcc/reload1.c	(working copy)
*************** eliminate_regs_1 (rtx x, enum machine_mo
*** 2548,2553 ****
--- 2548,2577 ----
      case POST_INC:
      case PRE_DEC:
      case POST_DEC:
+       /* We do not support elimination of a register that is modified.
+ 	 elimination_effects has already make sure that this does not
+ 	 happen.  */
+       return x;
+ 
+     case PRE_MODIFY:
+     case POST_MODIFY:
+       /* We do not support elimination of a register that is modified.
+ 	 elimination_effects has already make sure that this does not
+ 	 happen.  The only remaining case we need to consider here is
+ 	 that the increment value may be an eliminable register.  */
+       if (GET_CODE (XEXP (x, 1)) == PLUS
+ 	  && XEXP (XEXP (x, 1), 0) == XEXP (x, 0))
+ 	{
+ 	  rtx new = eliminate_regs_1 (XEXP (XEXP (x, 1), 1), mem_mode,
+ 				      insn, true);
+ 
+ 	  if (new != XEXP (XEXP (x, 1), 1))
+ 	    return gen_rtx_fmt_ee (code, GET_MODE (x), XEXP (x, 0),
+ 				   gen_rtx_PLUS (GET_MODE (x),
+ 						 XEXP (x, 0), new));
+ 	}
+       return x;
+ 
      case STRICT_LOW_PART:
      case NEG:          case NOT:
      case SIGN_EXTEND:  case ZERO_EXTEND:
*************** elimination_effects (rtx x, enum machine
*** 2743,2748 ****
--- 2767,2780 ----
      case POST_DEC:
      case POST_MODIFY:
      case PRE_MODIFY:
+       /* If we modify the source of an elimination rule, disable it.  */
+       for (ep = reg_eliminate; ep < &reg_eliminate[NUM_ELIMINABLE_REGS]; ep++)
+ 	if (ep->from_rtx == XEXP (x, 0))
+ 	  ep->can_eliminate = 0;
+ 
+       /* If we modify the target of an elimination rule by adding a constant,
+ 	 update its offset.  If we modify the target in any other way, we'll
+ 	 have to disable the rule as well.  */
        for (ep = reg_eliminate; ep < &reg_eliminate[NUM_ELIMINABLE_REGS]; ep++)
  	if (ep->to_rtx == XEXP (x, 0))
  	  {
*************** elimination_effects (rtx x, enum machine
*** 2757,2767 ****
  	      ep->offset += size;
  	    else if (code == PRE_INC || code == POST_INC)
  	      ep->offset -= size;
! 	    else if ((code == PRE_MODIFY || code == POST_MODIFY)
! 		     && GET_CODE (XEXP (x, 1)) == PLUS
! 		     && XEXP (x, 0) == XEXP (XEXP (x, 1), 0)
! 		     && CONSTANT_P (XEXP (XEXP (x, 1), 1)))
! 	      ep->offset -= INTVAL (XEXP (XEXP (x, 1), 1));
  	  }
  
        /* These two aren't unary operators.  */
--- 2789,2803 ----
  	      ep->offset += size;
  	    else if (code == PRE_INC || code == POST_INC)
  	      ep->offset -= size;
! 	    else if (code == PRE_MODIFY || code == POST_MODIFY)
! 	      {
! 		if (GET_CODE (XEXP (x, 1)) == PLUS
! 		    && XEXP (x, 0) == XEXP (XEXP (x, 1), 0)
! 		    && CONST_INT_P (XEXP (XEXP (x, 1), 1)))
! 		  ep->offset -= INTVAL (XEXP (XEXP (x, 1), 1));
! 		else
! 		  ep->can_eliminate = 0;
! 	      }
  	  }
  
        /* These two aren't unary operators.  */
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


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