[gcc(refs/users/aoliva/heads/testme)] -finline-stringops: check base blksize for memset [PR112778]

[Alexandre Oliva <aoliva@gcc.gnu.org>]

https://gcc.gnu.org/g:52537a3b3e1507fa6d121eb6d39a06804243a8ad

commit 52537a3b3e1507fa6d121eb6d39a06804243a8ad
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Fri Dec 8 21:41:47 2023 -0300

    -finline-stringops: check base blksize for memset [PR112778]
    
    The recently-added logic for -finline-stringops=memset introduced an
    assumption that doesn't necessarily hold, namely, that
    can_store_by_pieces of a larger size implies can_store_by_pieces by
    smaller sizes.  Checks for all sizes the by-multiple-pieces machinery
    might use before committing to an expansion pattern.
    
    
    for  gcc/ChangeLog
    
            PR target/112778
            * builtins.cc (can_store_by_multiple_pieces): New.
            (try_store_by_multiple_pieces): Call it.
    
    for  gcc/testsuite/ChangeLog
    
            PR target/112778
            * gcc.dg/inline-mem-cmp-pr112778.c: New.

Diff:
---
 gcc/builtins.cc                                | 57 ++++++++++++++++++++++----
 gcc/expr.cc                                    | 16 +++-----
 gcc/testsuite/gcc.dg/inline-mem-cmp-pr112778.c | 10 +++++
 3 files changed, 63 insertions(+), 20 deletions(-)

diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index 12a535d313f..f6c96498f07 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -4284,6 +4284,40 @@ expand_builtin_memset (tree exp, rtx target, machine_mode mode)
   return expand_builtin_memset_args (dest, val, len, target, mode, exp);
 }
 
+/* Check that store_by_pieces allows BITS + LEN (so that we don't
+   expand something too unreasonably long), and every power of 2 in
+   BITS.  It is assumed that LEN has already been tested by
+   itself.  */
+static bool
+can_store_by_multiple_pieces (unsigned HOST_WIDE_INT bits,
+			      by_pieces_constfn constfun,
+			      void *constfundata, unsigned int align,
+			      bool memsetp,
+			      unsigned HOST_WIDE_INT len)
+{
+  if (bits
+      && !can_store_by_pieces (bits + len, constfun, constfundata,
+			       align, memsetp))
+    return false;
+
+  /* BITS set are expected to be generally in the low range and
+     contiguous.  We do NOT want to repeat the test above in case BITS
+     has a single bit set, so we terminate the loop when BITS == BIT.
+     In the unlikely case that BITS has the MSB set, also terminate in
+     case BIT gets shifted out.  */
+  for (unsigned HOST_WIDE_INT bit = 1; bit < bits && bit; bit <<= 1)
+    {
+      if ((bits & bit) == 0)
+	continue;
+
+      if (!can_store_by_pieces (bit, constfun, constfundata,
+				align, memsetp))
+	return false;
+    }
+
+  return true;
+}
+
 /* Try to store VAL (or, if NULL_RTX, VALC) in LEN bytes starting at TO.
    Return TRUE if successful, FALSE otherwise.  TO is assumed to be
    aligned at an ALIGN-bits boundary.  LEN must be a multiple of
@@ -4341,7 +4375,11 @@ try_store_by_multiple_pieces (rtx to, rtx len, unsigned int ctz_len,
   else
     /* Huh, max_len < min_len?  Punt.  See pr100843.c.  */
     return false;
-  if (min_len >= blksize)
+  if (min_len >= blksize
+      /* ??? Maybe try smaller fixed-prefix blksizes before
+	 punting?  */
+      && can_store_by_pieces (blksize, builtin_memset_read_str,
+			      &valc, align, true))
     {
       min_len -= blksize;
       min_bits = floor_log2 (min_len);
@@ -4367,8 +4405,9 @@ try_store_by_multiple_pieces (rtx to, rtx len, unsigned int ctz_len,
      happen because of the way max_bits and blksize are related, but
      it doesn't hurt to test.  */
   if (blksize > xlenest
-      || !can_store_by_pieces (xlenest, builtin_memset_read_str,
-			       &valc, align, true))
+      || !can_store_by_multiple_pieces (xlenest - blksize,
+					builtin_memset_read_str,
+					&valc, align, true, blksize))
     {
       if (!(flag_inline_stringops & ILSOP_MEMSET))
 	return false;
@@ -4386,17 +4425,17 @@ try_store_by_multiple_pieces (rtx to, rtx len, unsigned int ctz_len,
 	     of overflow.  */
 	  if (max_bits < orig_max_bits
 	      && xlenest + blksize >= xlenest
-	      && can_store_by_pieces (xlenest + blksize,
-				      builtin_memset_read_str,
-				      &valc, align, true))
+	      && can_store_by_multiple_pieces (xlenest,
+					       builtin_memset_read_str,
+					       &valc, align, true, blksize))
 	    {
 	      max_loop = true;
 	      break;
 	    }
 	  if (blksize
-	      && can_store_by_pieces (xlenest,
-				      builtin_memset_read_str,
-				      &valc, align, true))
+	      && can_store_by_multiple_pieces (xlenest,
+					       builtin_memset_read_str,
+					       &valc, align, true, 0))
 	    {
 	      max_len += blksize;
 	      min_len += blksize;
diff --git a/gcc/expr.cc b/gcc/expr.cc
index 178b3ec6d5a..076ba706537 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -2451,15 +2451,12 @@ emit_block_move_via_loop (rtx x, rtx y, rtx size,
 
   opt_scalar_int_mode int_move_mode
     = int_mode_for_size (incr * BITS_PER_UNIT, 1);
-  if (!int_move_mode.exists ()
-      || (GET_MODE_BITSIZE (as_a <scalar_int_mode> (int_move_mode))
-	  != incr * BITS_PER_UNIT))
+  if (!int_move_mode.exists (&move_mode)
+      || GET_MODE_BITSIZE (int_move_mode.require ()) != incr * BITS_PER_UNIT)
     {
       move_mode = BLKmode;
       gcc_checking_assert (can_move_by_pieces (incr, align));
     }
-  else
-    move_mode = as_a <scalar_int_mode> (int_move_mode);
 
   x_addr = force_operand (XEXP (x, 0), NULL_RTX);
   y_addr = force_operand (XEXP (y, 0), NULL_RTX);
@@ -2705,16 +2702,13 @@ emit_block_cmp_via_loop (rtx x, rtx y, rtx len, tree len_type, rtx target,
 
   opt_scalar_int_mode int_cmp_mode
     = int_mode_for_size (incr * BITS_PER_UNIT, 1);
-  if (!int_cmp_mode.exists ()
-      || (GET_MODE_BITSIZE (as_a <scalar_int_mode> (int_cmp_mode))
-	  != incr * BITS_PER_UNIT)
-      || !can_compare_p (NE, as_a <scalar_int_mode> (int_cmp_mode), ccp_jump))
+  if (!int_cmp_mode.exists (&cmp_mode)
+      || GET_MODE_BITSIZE (int_cmp_mode.require ()) != incr * BITS_PER_UNIT
+      || !can_compare_p (NE, cmp_mode, ccp_jump))
     {
       cmp_mode = BLKmode;
       gcc_checking_assert (incr != 1);
     }
-  else
-    cmp_mode = as_a <scalar_int_mode> (int_cmp_mode);
 
   /* Save the base addresses.  */
   x_addr = force_operand (XEXP (x, 0), NULL_RTX);
diff --git a/gcc/testsuite/gcc.dg/inline-mem-cmp-pr112778.c b/gcc/testsuite/gcc.dg/inline-mem-cmp-pr112778.c
new file mode 100644
index 00000000000..fdfc5b6f28c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/inline-mem-cmp-pr112778.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-finline-stringops" } */
+
+char buf[3];
+
+int
+f ()
+{
+  __builtin_memset (buf, 'v', 3);
+}