Re: [patch]: COMMITTED Enhance rtl-dse to remove unaligned read after writes.

[Richard Sandiford <rsandifo at nildram dot co dot uk>]

Richard Sandiford <rsandifo@nildram.co.uk> writes:
> We should also skip access_sizes that are smaller than the store mode
> if truncating the store mode to the access size needs real insns.
> The patch below does this with:
>
>       /* Try a wider mode if truncating the store mode to ACCESS_SIZE
> 	 bytes requires a real instruction.  */
>       if (access_size < GET_MODE_SIZE (store_mode)
> 	  && !TRULY_NOOP_TRUNCATION (access_size * BITS_PER_UNIT,
> 				     GET_MODE_BITSIZE (store_mode)))
> 	continue;
>
> (The use of "continue" is out of style with the loop's current control
> flow.  For ease of review, I'll send a follow-up patch to fix that,
> with no behavioural changes.)

Here's that patch.  I'm no big fan of "continue", but it's the general
gcc style, and it avoids some overly-long lines in the current code.

Bootstrapped & regression-tested on x86_64-linux-gnu and
regression-tested on mipsisa64-elfoabi.  OK to install?

Richard


gcc/
	* dse.c (find_shift_sequence): No-op rework of control flow.

Index: gcc/dse.c
===================================================================
*** gcc/dse.c	2007-09-15 21:12:36.000000000 +0100
--- gcc/dse.c	2007-09-15 21:14:07.000000000 +0100
*************** find_shift_sequence (rtx read_reg,
*** 1409,1416 ****
  
    for (; access_size <= UNITS_PER_WORD; access_size *= 2)
      {
!       rtx target, new_reg;
        enum machine_mode new_mode;
  
        /* Try a wider mode if truncating the store mode to ACCESS_SIZE
  	 bytes requires a real instruction.  */
--- 1409,1417 ----
  
    for (; access_size <= UNITS_PER_WORD; access_size *= 2)
      {
!       rtx target, new_reg, shift_seq, insn;
        enum machine_mode new_mode;
+       int cost;
  
        /* Try a wider mode if truncating the store mode to ACCESS_SIZE
  	 bytes requires a real instruction.  */
*************** find_shift_sequence (rtx read_reg,
*** 1433,1496 ****
  			     GEN_INT (shift), new_reg, 1, OPTAB_DIRECT);
        df_clear_flags (DF_NO_INSN_RESCAN);
  
!       if (target == new_reg)
! 	{
! 	  rtx shift_seq = get_insns ();
! 	  end_sequence ();
  
! 	  /* If cost is too great, set target to NULL and
! 	     let the iteration happen. */
! 	  if (shift_seq != NULL)
! 	    {
! 	      int cost = 0;
! 	      rtx insn;
! 
! 	      for (insn = shift_seq; insn != NULL_RTX; insn = NEXT_INSN (insn))
! 		if (INSN_P (insn))
! 		  cost += insn_rtx_cost (PATTERN (insn));
! 
! 	      /* The computation up to here is essentially independent
! 		 of the arguments and could be precomputed.  It may
! 		 not be worth doing so.  We could precompute if
! 		 worthwhile or at least cache the results.  The result
! 		 technically depends on SHIFT, ACCESS_SIZE, and
! 		 GET_MODE_CLASS (READ_MODE).  But in practice the
! 		 answer will depend only on ACCESS_SIZE.  */
  
! 	      if (cost <= COSTS_N_INSNS (1))
! 		{
! 		  /* We found an acceptable shift.  Generate a move to
! 		     take the value from the store and put it into the
! 		     shift pseudo, then shift it, then generate another
! 		     move to put in into the target of the read.  */
! 		  start_sequence ();
! 		  emit_move_insn (new_reg, gen_lowpart (new_mode, store_info->rhs));
! 		  emit_insn (shift_seq);
! 		  convert_move (read_reg, new_reg, 1);
  		  
! 		  if (dump_file)
! 		    {
! 		      fprintf (dump_file, " -- adding extract insn r%d:%s = r%d:%s\n",
! 			       REGNO (new_reg), GET_MODE_NAME (new_mode),
! 			       REGNO (store_info->rhs), GET_MODE_NAME (store_mode));
  		      
! 		      fprintf (dump_file, " -- with shift of r%d by %d\n",
! 			       REGNO(new_reg), shift);
! 		      fprintf (dump_file, " -- and second extract insn r%d:%s = r%d:%s\n",
! 			       REGNO (read_reg), GET_MODE_NAME (read_mode),
! 			       REGNO (new_reg), GET_MODE_NAME (new_mode));
! 		    }
! 		  
! 		  /* Get the three insn sequence and return it.  */
! 		  shift_seq = get_insns ();
! 		  end_sequence ();
! 		  return shift_seq;
! 		}
! 	    }
  	}
!       else
! 	/* End the sequence.  */
! 	end_sequence ();
      }
  
    return NULL;
--- 1434,1487 ----
  			     GEN_INT (shift), new_reg, 1, OPTAB_DIRECT);
        df_clear_flags (DF_NO_INSN_RESCAN);
  
!       shift_seq = get_insns ();
!       end_sequence ();
  
!       if (target != new_reg || shift_seq == NULL)
! 	continue;
  
!       cost = 0;
!       for (insn = shift_seq; insn != NULL_RTX; insn = NEXT_INSN (insn))
! 	if (INSN_P (insn))
! 	  cost += insn_rtx_cost (PATTERN (insn));
! 
!       /* The computation up to here is essentially independent
! 	 of the arguments and could be precomputed.  It may
! 	 not be worth doing so.  We could precompute if
! 	 worthwhile or at least cache the results.  The result
! 	 technically depends on SHIFT, ACCESS_SIZE, and
! 	 GET_MODE_CLASS (READ_MODE).  But in practice the
! 	 answer will depend only on ACCESS_SIZE.  */
! 
!       if (cost > COSTS_N_INSNS (1))
! 	continue;
! 
!       /* We found an acceptable shift.  Generate a move to
! 	 take the value from the store and put it into the
! 	 shift pseudo, then shift it, then generate another
! 	 move to put in into the target of the read.  */
!       start_sequence ();
!       emit_move_insn (new_reg, gen_lowpart (new_mode, store_info->rhs));
!       emit_insn (shift_seq);
!       convert_move (read_reg, new_reg, 1);
  		  
!       if (dump_file)
! 	{
! 	  fprintf (dump_file, " -- adding extract insn r%d:%s = r%d:%s\n",
! 		   REGNO (new_reg), GET_MODE_NAME (new_mode),
! 		   REGNO (store_info->rhs), GET_MODE_NAME (store_mode));
  		      
! 	  fprintf (dump_file, " -- with shift of r%d by %d\n",
! 		   REGNO(new_reg), shift);
! 	  fprintf (dump_file, " -- and second extract insn r%d:%s = r%d:%s\n",
! 		   REGNO (read_reg), GET_MODE_NAME (read_mode),
! 		   REGNO (new_reg), GET_MODE_NAME (new_mode));
  	}
! 		  
!       /* Get the three insn sequence and return it.  */
!       shift_seq = get_insns ();
!       end_sequence ();
!       return shift_seq;
      }
  
    return NULL;