Re: [dataflow] remove try_auto_increment from regmove

[Kenneth Zadeck <zadeck at naturalbridge dot com>]

Paolo Bonzini wrote:
> Probably thanks to the improved auto-inc-dec pass on dataflow branch,
> the attached patch showed no assembly change whatsoever on
> powerpc-apple-darwin for the SPEC2000 sources.  I'm committing it in
> one or two days after bootstrap/regtest on i686-pc-linux-gnu and
> unless anybody convinces me of the opposite.
>
> 1 file changed, 12 insertions(+), 230 deletions(-)
>
> Thanks to Steven for the hint.
>
> Paolo
>
> ------------------------------------------------------------------------
>
> 2006-11-05  Paolo Bonzini  <bonzini@gnu.org>
>
> 	* regmove.c (find_use_as_address, try_auto_increment): Remove.
> 	(fixup_match_1, fixup_match_2): Remove calls and code that is now dead.
>
> Index: regmove.c
> ===================================================================
> --- regmove.c	(revision 118504)
> +++ regmove.c	(working copy)
> @@ -73,7 +73,6 @@ static rtx discover_flags_reg (void);
>  static void mark_flags_life_zones (struct df *, rtx);
>  static void flags_set_1 (rtx, rtx, void *);
>  
> -static int try_auto_increment (rtx, rtx, rtx, rtx, HOST_WIDE_INT, int);
>  static int find_matches (rtx, struct match *);
>  static void replace_in_call_usage (rtx *, unsigned int, rtx, rtx);
>  static int fixup_match_1 (rtx, rtx, rtx, rtx, rtx, int, int, int);
> @@ -94,133 +93,6 @@ regclass_compatible_p (int class0, int c
>  	      && ! CLASS_LIKELY_SPILLED_P (class1)));
>  }
>  
> -/* Find the place in the rtx X where REG is used as a memory address.
> -   Return the MEM rtx that so uses it.
> -   If PLUSCONST is nonzero, search instead for a memory address equivalent to
> -   (plus REG (const_int PLUSCONST)).
> -
> -   If such an address does not appear, return 0.
> -   If REG appears more than once, or is used other than in such an address,
> -   return (rtx) 1.  */
> -
> -static rtx
> -find_use_as_address (rtx x, rtx reg, HOST_WIDE_INT plusconst)
> -{
> -  enum rtx_code code = GET_CODE (x);
> -  const char * const fmt = GET_RTX_FORMAT (code);
> -  int i;
> -  rtx value = 0;
> -  rtx tem;
> -
> -  if (code == MEM && XEXP (x, 0) == reg && plusconst == 0)
> -    return x;
> -
> -  if (code == MEM && GET_CODE (XEXP (x, 0)) == PLUS
> -      && XEXP (XEXP (x, 0), 0) == reg
> -      && GET_CODE (XEXP (XEXP (x, 0), 1)) == CONST_INT
> -      && INTVAL (XEXP (XEXP (x, 0), 1)) == plusconst)
> -    return x;
> -
> -  if (code == SIGN_EXTRACT || code == ZERO_EXTRACT)
> -    {
> -      /* If REG occurs inside a MEM used in a bit-field reference,
> -	 that is unacceptable.  */
> -      if (find_use_as_address (XEXP (x, 0), reg, 0) != 0)
> -	return (rtx) (size_t) 1;
> -    }
> -
> -  if (x == reg)
> -    return (rtx) (size_t) 1;
> -
> -  for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
> -    {
> -      if (fmt[i] == 'e')
> -	{
> -	  tem = find_use_as_address (XEXP (x, i), reg, plusconst);
> -	  if (value == 0)
> -	    value = tem;
> -	  else if (tem != 0)
> -	    return (rtx) (size_t) 1;
> -	}
> -      else if (fmt[i] == 'E')
> -	{
> -	  int j;
> -	  for (j = XVECLEN (x, i) - 1; j >= 0; j--)
> -	    {
> -	      tem = find_use_as_address (XVECEXP (x, i, j), reg, plusconst);
> -	      if (value == 0)
> -		value = tem;
> -	      else if (tem != 0)
> -		return (rtx) (size_t) 1;
> -	    }
> -	}
> -    }
> -
> -  return value;
> -}
> -
> -
> -/* INC_INSN is an instruction that adds INCREMENT to REG.
> -   Try to fold INC_INSN as a post/pre in/decrement into INSN.
> -   Iff INC_INSN_SET is nonzero, inc_insn has a destination different from src.
> -   Return nonzero for success.  */
> -static int
> -try_auto_increment (rtx insn, rtx inc_insn, rtx inc_insn_set, rtx reg,
> -		    HOST_WIDE_INT increment, int pre)
> -{
> -  enum rtx_code inc_code;
> -
> -  rtx pset = single_set (insn);
> -  if (pset)
> -    {
> -      /* Can't use the size of SET_SRC, we might have something like
> -	 (sign_extend:SI (mem:QI ...  */
> -      rtx use = find_use_as_address (pset, reg, 0);
> -      if (use != 0 && use != (rtx) (size_t) 1)
> -	{
> -	  int size = GET_MODE_SIZE (GET_MODE (use));
> -	  if (0
> -	      || (HAVE_POST_INCREMENT
> -		  && pre == 0 && (inc_code = POST_INC, increment == size))
> -	      || (HAVE_PRE_INCREMENT
> -		  && pre == 1 && (inc_code = PRE_INC, increment == size))
> -	      || (HAVE_POST_DECREMENT
> -		  && pre == 0 && (inc_code = POST_DEC, increment == -size))
> -	      || (HAVE_PRE_DECREMENT
> -		  && pre == 1 && (inc_code = PRE_DEC, increment == -size))
> -	  )
> -	    {
> -	      if (inc_insn_set)
> -		validate_change
> -		  (inc_insn,
> -		   &SET_SRC (inc_insn_set),
> -		   XEXP (SET_SRC (inc_insn_set), 0), 1);
> -	      validate_change (insn, &XEXP (use, 0),
> -			       gen_rtx_fmt_e (inc_code, Pmode, reg), 1);
> -	      if (apply_change_group ())
> -		{
> -		  /* If there is a REG_DEAD note on this insn, we must
> -		     change this not to REG_UNUSED meaning that the register
> -		     is set, but the value is dead.  Failure to do so will
> -		     result in a sched1 dieing -- when it recomputes lifetime
> -		     information, the number of REG_DEAD notes will have
> -		     changed.  */
> -		  rtx note = find_reg_note (insn, REG_DEAD, reg);
> -		  if (note)
> -		    PUT_MODE (note, REG_UNUSED);
> -
> -		  REG_NOTES (insn)
> -		    = gen_rtx_EXPR_LIST (REG_INC,
> -					 reg, REG_NOTES (insn));
> -		  if (! inc_insn_set)
> -		    delete_insn (inc_insn);
> -		  return 1;
> -		}
> -	    }
> -	}
> -    }
> -  return 0;
> -}
>  
>  /* Determine if the pattern generated by add_optab has a clobber,
>     such as might be issued for a flags hard register.  To make the
> @@ -1048,35 +920,6 @@ fixup_match_2 (rtx insn, rtx dst, rtx sr
>  			 "Fixed operand of insn %d.\n",
>  			  INSN_UID (insn));
>  
> -#ifdef AUTO_INC_DEC
> -	      for (p = PREV_INSN (insn); p; p = PREV_INSN (p))
> -		{
> -		  if (LABEL_P (p)
> -		      || JUMP_P (p))
> -		    break;
> -		  if (! INSN_P (p))
> -		    continue;
> -		  if (reg_overlap_mentioned_p (dst, PATTERN (p)))
> -		    {
> -		      if (try_auto_increment (p, insn, 0, dst, newconst, 0))
> -			return 1;
> -		      break;
> -		    }
> -		}
> -	      for (p = NEXT_INSN (insn); p; p = NEXT_INSN (p))
> -		{
> -		  if (LABEL_P (p)
> -		      || JUMP_P (p))
> -		    break;
> -		  if (! INSN_P (p))
> -		    continue;
> -		  if (reg_overlap_mentioned_p (dst, PATTERN (p)))
> -		    {
> -		      try_auto_increment (p, insn, 0, dst, newconst, 1);
> -		      break;
> -		    }
> -		}
> -#endif
>  	      return 1;
>  	    }
>  	}
> @@ -1731,7 +1574,7 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
>  	       int backward, int operand_number, int match_number)
>  {
>    rtx p;
> -  rtx post_inc = 0, post_inc_set = 0, search_end = 0;
> +  rtx search_end = 0;
>    int success = 0;
>    int num_calls = 0, s_num_calls = 0;
>    enum rtx_code code = NOTE;
> @@ -1885,12 +1728,6 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
>  			      >= (GET_MODE_BITSIZE (GET_MODE
>  						    (SET_SRC (set2)))))))
>  		    break;
> -		  if (code == PLUS)
> -		    {
> -		      post_inc = q;
> -		      if (SET_DEST (set2) != src)
> -			post_inc_set = set2;
> -		    }
>  		}
>  	      /* We use 1 as last argument to validate_change so that all
>  		 changes are accepted or rejected together by apply_change_group
> @@ -1939,11 +1776,7 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
>    remove_note (p, dst_note);
>    if (code == MINUS)
>      {
> -      post_inc = emit_insn_after (copy_rtx (PATTERN (insn)), p);
> -      if ((HAVE_PRE_INCREMENT || HAVE_PRE_DECREMENT)
> -	  && search_end
> -	  && try_auto_increment (search_end, post_inc, 0, src, newconst, 1))
> -	post_inc = 0;
> +      emit_insn_after (copy_rtx (PATTERN (insn)), p);
>        validate_change (insn, &XEXP (SET_SRC (set), 1), GEN_INT (insn_const), 0);
>        REG_N_SETS (REGNO (src))++;
>        REG_LIVE_LENGTH (REGNO (src))++;
> @@ -1953,26 +1786,20 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
>        /* The lifetime of src and dest overlap,
>  	 but we can change this by moving insn.  */
>        rtx pat = PATTERN (insn);
> +      rtx notes;
>        if (src_note)
>  	remove_note (overlap, src_note);
> -      if ((HAVE_POST_INCREMENT || HAVE_POST_DECREMENT)
> -	  && code == PLUS
> -	  && try_auto_increment (overlap, insn, 0, src, insn_const, 0))
> -	insn = overlap;
> -      else
> -	{
> -	  rtx notes = REG_NOTES (insn);
>  
> -	  emit_insn_after_with_line_notes (pat, PREV_INSN (p), insn);
> -	  delete_insn (insn);
> -	  /* emit_insn_after_with_line_notes has no
> -	     return value, so search for the new insn.  */
> -	  insn = p;
> -	  while (! INSN_P (insn) || PATTERN (insn) != pat)
> -	    insn = PREV_INSN (insn);
> +      notes = REG_NOTES (insn);
> +      emit_insn_after_with_line_notes (pat, PREV_INSN (p), insn);
> +      delete_insn (insn);
> +      /* emit_insn_after_with_line_notes has no
> +         return value, so search for the new insn.  */
> +      insn = p;
> +      while (! INSN_P (insn) || PATTERN (insn) != pat)
> +        insn = PREV_INSN (insn);
>  
> -	  REG_NOTES (insn) = notes;
> -	}
> +      REG_NOTES (insn) = notes;
>      }
>    /* Sometimes we'd generate src = const; src += n;
>       if so, replace the instruction that set src
> @@ -2025,51 +1852,6 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
>  	}
>      }
>  
> -  if ((HAVE_PRE_INCREMENT || HAVE_PRE_DECREMENT)
> -	   && (code == PLUS || code == MINUS) && insn_const
> -	   && try_auto_increment (p, insn, 0, src, insn_const, 1))
> -    insn = p;
> -  else if ((HAVE_POST_INCREMENT || HAVE_POST_DECREMENT)
> -	   && post_inc
> -	   && try_auto_increment (p, post_inc, post_inc_set, src, newconst, 0))
> -    post_inc = 0;
> -  /* If post_inc still prevails, try to find an
> -     insn where it can be used as a pre-in/decrement.
> -     If code is MINUS, this was already tried.  */
> -  if (post_inc && code == PLUS
> -  /* Check that newconst is likely to be usable
> -     in a pre-in/decrement before starting the search.  */
> -      && ((HAVE_PRE_INCREMENT && newconst > 0 && newconst <= MOVE_MAX)
> -	  || (HAVE_PRE_DECREMENT && newconst < 0 && newconst >= -MOVE_MAX))
> -      && exact_log2 (newconst))
> -    {
> -      rtx q, inc_dest;
> -
> -      inc_dest = post_inc_set ? SET_DEST (post_inc_set) : src;
> -      for (q = post_inc; (q = NEXT_INSN (q)); )
> -	{
> -	  /* ??? We can't scan past the end of a basic block without updating
> -	     the register lifetime info
> -	     (REG_DEAD/basic_block_live_at_start).  */
> -	  if (perhaps_ends_bb_p (q))
> -	    break;
> -	  else if (! INSN_P (q))
> -	    continue;
> -	  else if (src != inc_dest
> -		   && (reg_overlap_mentioned_p (src, PATTERN (q))
> -		       || reg_set_p (src, q)))
> -	    break;
> -	  else if (reg_set_p (inc_dest, q))
> -	    break;
> -	  else if (reg_overlap_mentioned_p (inc_dest, PATTERN (q)))
> -	    {
> -	      try_auto_increment (q, post_inc,
> -				  post_inc_set, inc_dest, newconst, 1);
> -	      break;
> -	    }
> -	}
> -    }
> -
>    /* Move the death note for DST to INSN if it is used
>       there.  */
>    if (reg_overlap_mentioned_p (dst, PATTERN (insn)))
>   
I think if we either had the arm or the ia-64 working I would agree to
this after you bootstrap on those platforms and cross checked the spec
benchmarks on those platforms.  But the x86 does not have any auto
increment instructions so you are playing air guitar here.

If one of you would like to get the dataflow branch working on one of
those platforms we would love the help.  But until then you need to hold
off on this patch.