[Patch] Cleanup widest_int_mode_for_size

James Greenhalgh james.greenhalgh@arm.com
Tue Sep 30 08:37:00 GMT 2014


*ping*

Thanks,
James

On Tue, Sep 23, 2014 at 10:17:21AM +0100, James Greenhalgh wrote:
> 
> Hi,
> 
> The comment on widest_int_mode_for_size claims that it returns the
> widest integer mode no wider than size. The implementation looks more
> like it finds the widest integer mode smaller than size. Everywhere it
> is used, the mode it is looking for is ultimately checked against an
> expected alignment or is used for heuristics that should be thinking
> about that check, so pull it in to here.
> 
> Throughout expr.c corrections are made for this fact - adding one to
> the size passed to this function. This feels a bit backwards to me.
> 
> This patch fixes that, and then fixes the fallout throughout expr.c.
> Generally, this means simplifying a bunch of move_by_pieces style copy
> code.
> 
> Bootstrapped on x86_64, arm and AArch64 with no issues.
> 
> OK for trunk?
> 
> Thanks,
> James
> 
> 2014-09-23  James Greenhalgh  <james.greenhalgh@arm.com>
> 
> 	* expr.c (MOVE_BY_PIECES_P): Remove off-by-one correction to
> 	move_by_pieces_ninsns.
> 	(CLEAR_BY_PIECES_P): Likewise.
> 	(SET_BY_PIECES_P): Likewise.
> 	(STORE_BY_PIECES_P): Likwise.
> 	(widest_int_mode_for_size): Return the widest mode in which the
> 	given size fits.
> 	(move_by_pieces): Remove off-by-one correction for max_size,
> 	simplify copy loop body.
> 	(move_by_pieces_ninsns): Simplify copy body.
> 	(can_store_by_pieces): Remove off-by-one correction for max_size,
> 	simplify copy body.
> 	(store_by_pieces_1): Likewise.
> 

> diff --git a/gcc/expr.c b/gcc/expr.c
> index a6233f3..0af9b9a 100644
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -161,7 +161,7 @@ static void write_complex_part (rtx, rtx, bool);
>     to perform a structure copy.  */
>  #ifndef MOVE_BY_PIECES_P
>  #define MOVE_BY_PIECES_P(SIZE, ALIGN) \
> -  (move_by_pieces_ninsns (SIZE, ALIGN, MOVE_MAX_PIECES + 1) \
> +  (move_by_pieces_ninsns (SIZE, ALIGN, MOVE_MAX_PIECES) \
>     < (unsigned int) MOVE_RATIO (optimize_insn_for_speed_p ()))
>  #endif
>  
> @@ -169,7 +169,7 @@ static void write_complex_part (rtx, rtx, bool);
>     called to clear storage.  */
>  #ifndef CLEAR_BY_PIECES_P
>  #define CLEAR_BY_PIECES_P(SIZE, ALIGN) \
> -  (move_by_pieces_ninsns (SIZE, ALIGN, STORE_MAX_PIECES + 1) \
> +  (move_by_pieces_ninsns (SIZE, ALIGN, STORE_MAX_PIECES) \
>     < (unsigned int) CLEAR_RATIO (optimize_insn_for_speed_p ()))
>  #endif
>  
> @@ -177,7 +177,7 @@ static void write_complex_part (rtx, rtx, bool);
>     called to "memset" storage with byte values other than zero.  */
>  #ifndef SET_BY_PIECES_P
>  #define SET_BY_PIECES_P(SIZE, ALIGN) \
> -  (move_by_pieces_ninsns (SIZE, ALIGN, STORE_MAX_PIECES + 1) \
> +  (move_by_pieces_ninsns (SIZE, ALIGN, STORE_MAX_PIECES) \
>     < (unsigned int) SET_RATIO (optimize_insn_for_speed_p ()))
>  #endif
>  
> @@ -185,7 +185,7 @@ static void write_complex_part (rtx, rtx, bool);
>     called to "memcpy" storage when the source is a constant string.  */
>  #ifndef STORE_BY_PIECES_P
>  #define STORE_BY_PIECES_P(SIZE, ALIGN) \
> -  (move_by_pieces_ninsns (SIZE, ALIGN, STORE_MAX_PIECES + 1) \
> +  (move_by_pieces_ninsns (SIZE, ALIGN, STORE_MAX_PIECES) \
>     < (unsigned int) MOVE_RATIO (optimize_insn_for_speed_p ()))
>  #endif
>  
> @@ -801,18 +801,23 @@ alignment_for_piecewise_move (unsigned int max_pieces, unsigned int align)
>    return align;
>  }
>  
> -/* Return the widest integer mode no wider than SIZE.  If no such mode
> -   can be found, return VOIDmode.  */
> +/* Return the widest integer mode no wider than SIZE which can be accessed
> +   at the given ALIGNMENT.  If no such mode can be found, return VOIDmode.
> +   If SIZE would fit exactly in a mode, return that mode. */
>  
>  static enum machine_mode
> -widest_int_mode_for_size (unsigned int size)
> +widest_int_mode_for_size_and_alignment (unsigned int size,
> +					unsigned int align)
>  {
>    enum machine_mode tmode, mode = VOIDmode;
>  
>    for (tmode = GET_CLASS_NARROWEST_MODE (MODE_INT);
>         tmode != VOIDmode; tmode = GET_MODE_WIDER_MODE (tmode))
> -    if (GET_MODE_SIZE (tmode) < size)
> -      mode = tmode;
> +    {
> +      if (GET_MODE_SIZE (tmode) <= size
> +	  && align >= GET_MODE_ALIGNMENT (mode))
> +	mode = tmode;
> +    }
>  
>    return mode;
>  }
> @@ -855,7 +860,7 @@ move_by_pieces (rtx to, rtx from, unsigned HOST_WIDE_INT len,
>    enum machine_mode to_addr_mode;
>    enum machine_mode from_addr_mode = get_address_mode (from);
>    rtx to_addr, from_addr = XEXP (from, 0);
> -  unsigned int max_size = MOVE_MAX_PIECES + 1;
> +  unsigned int max_size = MOVE_MAX_PIECES;
>    enum insn_code icode;
>  
>    align = MIN (to ? MEM_ALIGN (to) : align, MEM_ALIGN (from));
> @@ -907,7 +912,7 @@ move_by_pieces (rtx to, rtx from, unsigned HOST_WIDE_INT len,
>  	 MODE might not be used depending on the definitions of the
>  	 USE_* macros below.  */
>        enum machine_mode mode ATTRIBUTE_UNUSED
> -	= widest_int_mode_for_size (max_size);
> +	= widest_int_mode_for_size_and_alignment (max_size, align);
>  
>        if (USE_LOAD_PRE_DECREMENT (mode) && data.reverse && ! data.autinc_from)
>  	{
> @@ -948,18 +953,20 @@ move_by_pieces (rtx to, rtx from, unsigned HOST_WIDE_INT len,
>    /* First move what we can in the largest integer mode, then go to
>       successively smaller modes.  */
>  
> -  while (max_size > 1 && data.len > 0)
> +  while (data.len > 0)
>      {
> -      enum machine_mode mode = widest_int_mode_for_size (max_size);
> +      enum machine_mode mode
> +	= widest_int_mode_for_size_and_alignment (MIN (max_size, data.len),
> +						  align);
>  
>        if (mode == VOIDmode)
>  	break;
>  
>        icode = optab_handler (mov_optab, mode);
> -      if (icode != CODE_FOR_nothing && align >= GET_MODE_ALIGNMENT (mode))
> +      if (icode != CODE_FOR_nothing)
>  	move_by_pieces_1 (GEN_FCN (icode), mode, &data);
> -
> -      max_size = GET_MODE_SIZE (mode);
> +      else
> +	max_size = GET_MODE_SIZE (mode) - 1;
>      }
>  
>    /* The code above should have handled everything.  */
> @@ -1008,21 +1015,25 @@ move_by_pieces_ninsns (unsigned HOST_WIDE_INT l, unsigned int align,
>  
>    align = alignment_for_piecewise_move (MOVE_MAX_PIECES, align);
>  
> -  while (max_size > 1 && l > 0)
> +  while (l > 0)
>      {
>        enum machine_mode mode;
>        enum insn_code icode;
>  
> -      mode = widest_int_mode_for_size (max_size);
> +      mode = widest_int_mode_for_size_and_alignment (MIN (max_size, l),
> +						     align);
>  
>        if (mode == VOIDmode)
>  	break;
>  
>        icode = optab_handler (mov_optab, mode);
> -      if (icode != CODE_FOR_nothing && align >= GET_MODE_ALIGNMENT (mode))
> -	n_insns += l / GET_MODE_SIZE (mode), l %= GET_MODE_SIZE (mode);
> -
> -      max_size = GET_MODE_SIZE (mode);
> +      if (icode != CODE_FOR_nothing)
> +	{
> +	  n_insns += l / GET_MODE_SIZE (mode);
> +	  l %= GET_MODE_SIZE (mode);
> +	}
> +      else
> +	max_size = GET_MODE_SIZE (mode) - 1;
>      }
>  
>    gcc_assert (!l);
> @@ -2475,10 +2486,10 @@ can_store_by_pieces (unsigned HOST_WIDE_INT len,
>  		     void *constfundata, unsigned int align, bool memsetp)
>  {
>    unsigned HOST_WIDE_INT l;
> -  unsigned int max_size;
>    HOST_WIDE_INT offset = 0;
>    enum machine_mode mode;
>    enum insn_code icode;
> +  unsigned int max_size = STORE_MAX_PIECES;
>    int reverse;
>    /* cst is set but not used if LEGITIMATE_CONSTANT doesn't use it.  */
>    rtx cst ATTRIBUTE_UNUSED;
> @@ -2501,17 +2512,16 @@ can_store_by_pieces (unsigned HOST_WIDE_INT len,
>         reverse++)
>      {
>        l = len;
> -      max_size = STORE_MAX_PIECES + 1;
> -      while (max_size > 1 && l > 0)
> +      while (l > 0)
>  	{
> -	  mode = widest_int_mode_for_size (max_size);
> +	  mode = widest_int_mode_for_size_and_alignment
> +		  (MIN (max_size, l), align);
>  
>  	  if (mode == VOIDmode)
>  	    break;
>  
>  	  icode = optab_handler (mov_optab, mode);
> -	  if (icode != CODE_FOR_nothing
> -	      && align >= GET_MODE_ALIGNMENT (mode))
> +	  if (icode != CODE_FOR_nothing)
>  	    {
>  	      unsigned int size = GET_MODE_SIZE (mode);
>  
> @@ -2530,8 +2540,8 @@ can_store_by_pieces (unsigned HOST_WIDE_INT len,
>  		  l -= size;
>  		}
>  	    }
> -
> -	  max_size = GET_MODE_SIZE (mode);
> +	  else
> +	    max_size = GET_MODE_SIZE (mode) - 1;
>  	}
>  
>        /* The code above should have handled everything.  */
> @@ -2643,7 +2653,7 @@ store_by_pieces_1 (struct store_by_pieces_d *data ATTRIBUTE_UNUSED,
>  {
>    enum machine_mode to_addr_mode = get_address_mode (data->to);
>    rtx to_addr = XEXP (data->to, 0);
> -  unsigned int max_size = STORE_MAX_PIECES + 1;
> +  unsigned int max_size = STORE_MAX_PIECES;
>    enum insn_code icode;
>  
>    data->offset = 0;
> @@ -2668,7 +2678,7 @@ store_by_pieces_1 (struct store_by_pieces_d *data ATTRIBUTE_UNUSED,
>  	 MODE might not be used depending on the definitions of the
>  	 USE_* macros below.  */
>        enum machine_mode mode ATTRIBUTE_UNUSED
> -	= widest_int_mode_for_size (max_size);
> +	= widest_int_mode_for_size_and_alignment (max_size, align);
>  
>        if (USE_STORE_PRE_DECREMENT (mode) && data->reverse && ! data->autinc_to)
>  	{
> @@ -2697,18 +2707,20 @@ store_by_pieces_1 (struct store_by_pieces_d *data ATTRIBUTE_UNUSED,
>    /* First store what we can in the largest integer mode, then go to
>       successively smaller modes.  */
>  
> -  while (max_size > 1 && data->len > 0)
> +  while (data->len > 0)
>      {
> -      enum machine_mode mode = widest_int_mode_for_size (max_size);
> +      enum machine_mode mode
> +	= widest_int_mode_for_size_and_alignment (MIN (max_size, data->len),
> +						  align);
>  
>        if (mode == VOIDmode)
>  	break;
>  
>        icode = optab_handler (mov_optab, mode);
> -      if (icode != CODE_FOR_nothing && align >= GET_MODE_ALIGNMENT (mode))
> +      if (icode != CODE_FOR_nothing)
>  	store_by_pieces_2 (GEN_FCN (icode), mode, data);
> -
> -      max_size = GET_MODE_SIZE (mode);
> +      else
> +	max_size = GET_MODE_SIZE (mode) - 1;
>      }
>  
>    /* The code above should have handled everything.  */



More information about the Gcc-patches mailing list