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]

[Patch] Cleanup widest_int_mode_for_size


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.  */

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