public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Simplify & expand c_readstr
@ 2023-09-28 13:36 Richard Sandiford
  2023-09-29  6:28 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Richard Sandiford @ 2023-09-28 13:36 UTC (permalink / raw)
  To: gcc-patches; +Cc: HAO CHEN GUI, Kewen.Lin

c_readstr only operated on integer modes.  It worked by reading
the source string into an array of HOST_WIDE_INTs, converting
that array into a wide_int, and from there to an rtx.

It's simpler to do this by building a target memory image and
using native_decode_rtx to convert that memory image into an rtx.
It avoids all the endianness shenanigans because both the string and
native_decode_rtx follow target memory order.  It also means that the
function can handle all fixed-size modes, which simplifies callers
and allows vector modes to be used more widely.

Tested on aarch64-linux-gnu so far.  OK to install?

Richard


gcc/
	* builtins.h (c_readstr): Take a fixed_size_mode rather than a
	scalar_int_mode.
	* builtins.cc (c_readstr): Likewise.  Build a local array of
	bytes and use native_decode_rtx to get the rtx image.
	(builtin_memcpy_read_str): Simplify accordingly.
	(builtin_strncpy_read_str): Likewise.
	(builtin_memset_read_str): Likewise.
	(builtin_memset_gen_str: Likewise.
	* expr.cc (string_cst_read_str): Likewise.
---
 gcc/builtins.cc | 46 +++++++++++-----------------------------------
 gcc/builtins.h  |  2 +-
 gcc/expr.cc     |  5 ++---
 3 files changed, 14 insertions(+), 39 deletions(-)

diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index 40dfd36a319..cb90bd03b3e 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -743,39 +743,22 @@ c_strlen (tree arg, int only_value, c_strlen_data *data, unsigned eltsize)
    as needed.  */
 
 rtx
-c_readstr (const char *str, scalar_int_mode mode,
+c_readstr (const char *str, fixed_size_mode mode,
 	   bool null_terminated_p/*=true*/)
 {
-  HOST_WIDE_INT ch;
-  unsigned int i, j;
-  HOST_WIDE_INT tmp[MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT];
+  auto_vec<target_unit, MAX_BITSIZE_MODE_ANY_INT / BITS_PER_UNIT> bytes;
 
-  gcc_assert (GET_MODE_CLASS (mode) == MODE_INT);
-  unsigned int len = (GET_MODE_PRECISION (mode) + HOST_BITS_PER_WIDE_INT - 1)
-    / HOST_BITS_PER_WIDE_INT;
+  bytes.reserve (GET_MODE_SIZE (mode));
 
-  gcc_assert (len <= MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT);
-  for (i = 0; i < len; i++)
-    tmp[i] = 0;
-
-  ch = 1;
-  for (i = 0; i < GET_MODE_SIZE (mode); i++)
+  target_unit ch = 1;
+  for (unsigned int i = 0; i < GET_MODE_SIZE (mode); ++i)
     {
-      j = i;
-      if (WORDS_BIG_ENDIAN)
-	j = GET_MODE_SIZE (mode) - i - 1;
-      if (BYTES_BIG_ENDIAN != WORDS_BIG_ENDIAN
-	  && GET_MODE_SIZE (mode) >= UNITS_PER_WORD)
-	j = j + UNITS_PER_WORD - 2 * (j % UNITS_PER_WORD) - 1;
-      j *= BITS_PER_UNIT;
-
       if (ch || !null_terminated_p)
 	ch = (unsigned char) str[i];
-      tmp[j / HOST_BITS_PER_WIDE_INT] |= ch << (j % HOST_BITS_PER_WIDE_INT);
+      bytes.quick_push (ch);
     }
 
-  wide_int c = wide_int::from_array (tmp, len, GET_MODE_PRECISION (mode));
-  return immed_wide_int_const (c, mode);
+  return native_decode_rtx (mode, bytes, 0);
 }
 
 /* Cast a target constant CST to target CHAR and if that value fits into
@@ -3530,10 +3513,7 @@ builtin_memcpy_read_str (void *data, void *, HOST_WIDE_INT offset,
      string but the caller guarantees it's large enough for MODE.  */
   const char *rep = (const char *) data;
 
-  /* The by-pieces infrastructure does not try to pick a vector mode
-     for memcpy expansion.  */
-  return c_readstr (rep + offset, as_a <scalar_int_mode> (mode),
-		    /*nul_terminated=*/false);
+  return c_readstr (rep + offset, mode, /*nul_terminated=*/false);
 }
 
 /* LEN specify length of the block of memcpy/memset operation.
@@ -3994,9 +3974,7 @@ builtin_strncpy_read_str (void *data, void *, HOST_WIDE_INT offset,
   if ((unsigned HOST_WIDE_INT) offset > strlen (str))
     return const0_rtx;
 
-  /* The by-pieces infrastructure does not try to pick a vector mode
-     for strncpy expansion.  */
-  return c_readstr (str + offset, as_a <scalar_int_mode> (mode));
+  return c_readstr (str + offset, mode);
 }
 
 /* Helper to check the sizes of sequences and the destination of calls
@@ -4227,8 +4205,7 @@ builtin_memset_read_str (void *data, void *prev,
 
   memset (p, *c, size);
 
-  /* Vector modes should be handled above.  */
-  return c_readstr (p, as_a <scalar_int_mode> (mode));
+  return c_readstr (p, mode);
 }
 
 /* Callback routine for store_by_pieces.  Return the RTL of a register
@@ -4275,8 +4252,7 @@ builtin_memset_gen_str (void *data, void *prev,
 
   p = XALLOCAVEC (char, size);
   memset (p, 1, size);
-  /* Vector modes should be handled above.  */
-  coeff = c_readstr (p, as_a <scalar_int_mode> (mode));
+  coeff = c_readstr (p, mode);
 
   target = convert_to_mode (mode, (rtx) data, 1);
   target = expand_mult (mode, target, coeff, NULL_RTX, 1);
diff --git a/gcc/builtins.h b/gcc/builtins.h
index 3b5c34c4802..88a26d70cd5 100644
--- a/gcc/builtins.h
+++ b/gcc/builtins.h
@@ -105,7 +105,7 @@ struct c_strlen_data
 };
 
 extern tree c_strlen (tree, int, c_strlen_data * = NULL, unsigned = 1);
-extern rtx c_readstr (const char *, scalar_int_mode, bool = true);
+extern rtx c_readstr (const char *, fixed_size_mode, bool = true);
 extern void expand_builtin_setjmp_setup (rtx, rtx);
 extern void expand_builtin_setjmp_receiver (rtx);
 extern void expand_builtin_update_setjmp_buf (rtx);
diff --git a/gcc/expr.cc b/gcc/expr.cc
index 308ddc09e63..a57948bdf01 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -6083,13 +6083,12 @@ string_cst_read_str (void *data, void *, HOST_WIDE_INT offset,
       size_t l = TREE_STRING_LENGTH (str) - offset;
       memcpy (p, TREE_STRING_POINTER (str) + offset, l);
       memset (p + l, '\0', GET_MODE_SIZE (mode) - l);
-      return c_readstr (p, as_a <scalar_int_mode> (mode), false);
+      return c_readstr (p, mode, false);
     }
 
   /* The by-pieces infrastructure does not try to pick a vector mode
      for storing STRING_CST.  */
-  return c_readstr (TREE_STRING_POINTER (str) + offset,
-		    as_a <scalar_int_mode> (mode), false);
+  return c_readstr (TREE_STRING_POINTER (str) + offset, mode, false);
 }
 
 /* Generate code for computing expression EXP,
-- 
2.25.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Simplify & expand c_readstr
  2023-09-28 13:36 [PATCH] Simplify & expand c_readstr Richard Sandiford
@ 2023-09-29  6:28 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2023-09-29  6:28 UTC (permalink / raw)
  To: gcc-patches, HAO CHEN GUI, Kewen.Lin, richard.sandiford

On Thu, Sep 28, 2023 at 3:37 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> c_readstr only operated on integer modes.  It worked by reading
> the source string into an array of HOST_WIDE_INTs, converting
> that array into a wide_int, and from there to an rtx.
>
> It's simpler to do this by building a target memory image and
> using native_decode_rtx to convert that memory image into an rtx.
> It avoids all the endianness shenanigans because both the string and
> native_decode_rtx follow target memory order.  It also means that the
> function can handle all fixed-size modes, which simplifies callers
> and allows vector modes to be used more widely.
>
> Tested on aarch64-linux-gnu so far.  OK to install?

OK.

Richard.

> Richard
>
>
> gcc/
>         * builtins.h (c_readstr): Take a fixed_size_mode rather than a
>         scalar_int_mode.
>         * builtins.cc (c_readstr): Likewise.  Build a local array of
>         bytes and use native_decode_rtx to get the rtx image.
>         (builtin_memcpy_read_str): Simplify accordingly.
>         (builtin_strncpy_read_str): Likewise.
>         (builtin_memset_read_str): Likewise.
>         (builtin_memset_gen_str: Likewise.
>         * expr.cc (string_cst_read_str): Likewise.
> ---
>  gcc/builtins.cc | 46 +++++++++++-----------------------------------
>  gcc/builtins.h  |  2 +-
>  gcc/expr.cc     |  5 ++---
>  3 files changed, 14 insertions(+), 39 deletions(-)
>
> diff --git a/gcc/builtins.cc b/gcc/builtins.cc
> index 40dfd36a319..cb90bd03b3e 100644
> --- a/gcc/builtins.cc
> +++ b/gcc/builtins.cc
> @@ -743,39 +743,22 @@ c_strlen (tree arg, int only_value, c_strlen_data *data, unsigned eltsize)
>     as needed.  */
>
>  rtx
> -c_readstr (const char *str, scalar_int_mode mode,
> +c_readstr (const char *str, fixed_size_mode mode,
>            bool null_terminated_p/*=true*/)
>  {
> -  HOST_WIDE_INT ch;
> -  unsigned int i, j;
> -  HOST_WIDE_INT tmp[MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT];
> +  auto_vec<target_unit, MAX_BITSIZE_MODE_ANY_INT / BITS_PER_UNIT> bytes;
>
> -  gcc_assert (GET_MODE_CLASS (mode) == MODE_INT);
> -  unsigned int len = (GET_MODE_PRECISION (mode) + HOST_BITS_PER_WIDE_INT - 1)
> -    / HOST_BITS_PER_WIDE_INT;
> +  bytes.reserve (GET_MODE_SIZE (mode));
>
> -  gcc_assert (len <= MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT);
> -  for (i = 0; i < len; i++)
> -    tmp[i] = 0;
> -
> -  ch = 1;
> -  for (i = 0; i < GET_MODE_SIZE (mode); i++)
> +  target_unit ch = 1;
> +  for (unsigned int i = 0; i < GET_MODE_SIZE (mode); ++i)
>      {
> -      j = i;
> -      if (WORDS_BIG_ENDIAN)
> -       j = GET_MODE_SIZE (mode) - i - 1;
> -      if (BYTES_BIG_ENDIAN != WORDS_BIG_ENDIAN
> -         && GET_MODE_SIZE (mode) >= UNITS_PER_WORD)
> -       j = j + UNITS_PER_WORD - 2 * (j % UNITS_PER_WORD) - 1;
> -      j *= BITS_PER_UNIT;
> -
>        if (ch || !null_terminated_p)
>         ch = (unsigned char) str[i];
> -      tmp[j / HOST_BITS_PER_WIDE_INT] |= ch << (j % HOST_BITS_PER_WIDE_INT);
> +      bytes.quick_push (ch);
>      }
>
> -  wide_int c = wide_int::from_array (tmp, len, GET_MODE_PRECISION (mode));
> -  return immed_wide_int_const (c, mode);
> +  return native_decode_rtx (mode, bytes, 0);
>  }
>
>  /* Cast a target constant CST to target CHAR and if that value fits into
> @@ -3530,10 +3513,7 @@ builtin_memcpy_read_str (void *data, void *, HOST_WIDE_INT offset,
>       string but the caller guarantees it's large enough for MODE.  */
>    const char *rep = (const char *) data;
>
> -  /* The by-pieces infrastructure does not try to pick a vector mode
> -     for memcpy expansion.  */
> -  return c_readstr (rep + offset, as_a <scalar_int_mode> (mode),
> -                   /*nul_terminated=*/false);
> +  return c_readstr (rep + offset, mode, /*nul_terminated=*/false);
>  }
>
>  /* LEN specify length of the block of memcpy/memset operation.
> @@ -3994,9 +3974,7 @@ builtin_strncpy_read_str (void *data, void *, HOST_WIDE_INT offset,
>    if ((unsigned HOST_WIDE_INT) offset > strlen (str))
>      return const0_rtx;
>
> -  /* The by-pieces infrastructure does not try to pick a vector mode
> -     for strncpy expansion.  */
> -  return c_readstr (str + offset, as_a <scalar_int_mode> (mode));
> +  return c_readstr (str + offset, mode);
>  }
>
>  /* Helper to check the sizes of sequences and the destination of calls
> @@ -4227,8 +4205,7 @@ builtin_memset_read_str (void *data, void *prev,
>
>    memset (p, *c, size);
>
> -  /* Vector modes should be handled above.  */
> -  return c_readstr (p, as_a <scalar_int_mode> (mode));
> +  return c_readstr (p, mode);
>  }
>
>  /* Callback routine for store_by_pieces.  Return the RTL of a register
> @@ -4275,8 +4252,7 @@ builtin_memset_gen_str (void *data, void *prev,
>
>    p = XALLOCAVEC (char, size);
>    memset (p, 1, size);
> -  /* Vector modes should be handled above.  */
> -  coeff = c_readstr (p, as_a <scalar_int_mode> (mode));
> +  coeff = c_readstr (p, mode);
>
>    target = convert_to_mode (mode, (rtx) data, 1);
>    target = expand_mult (mode, target, coeff, NULL_RTX, 1);
> diff --git a/gcc/builtins.h b/gcc/builtins.h
> index 3b5c34c4802..88a26d70cd5 100644
> --- a/gcc/builtins.h
> +++ b/gcc/builtins.h
> @@ -105,7 +105,7 @@ struct c_strlen_data
>  };
>
>  extern tree c_strlen (tree, int, c_strlen_data * = NULL, unsigned = 1);
> -extern rtx c_readstr (const char *, scalar_int_mode, bool = true);
> +extern rtx c_readstr (const char *, fixed_size_mode, bool = true);
>  extern void expand_builtin_setjmp_setup (rtx, rtx);
>  extern void expand_builtin_setjmp_receiver (rtx);
>  extern void expand_builtin_update_setjmp_buf (rtx);
> diff --git a/gcc/expr.cc b/gcc/expr.cc
> index 308ddc09e63..a57948bdf01 100644
> --- a/gcc/expr.cc
> +++ b/gcc/expr.cc
> @@ -6083,13 +6083,12 @@ string_cst_read_str (void *data, void *, HOST_WIDE_INT offset,
>        size_t l = TREE_STRING_LENGTH (str) - offset;
>        memcpy (p, TREE_STRING_POINTER (str) + offset, l);
>        memset (p + l, '\0', GET_MODE_SIZE (mode) - l);
> -      return c_readstr (p, as_a <scalar_int_mode> (mode), false);
> +      return c_readstr (p, mode, false);
>      }
>
>    /* The by-pieces infrastructure does not try to pick a vector mode
>       for storing STRING_CST.  */
> -  return c_readstr (TREE_STRING_POINTER (str) + offset,
> -                   as_a <scalar_int_mode> (mode), false);
> +  return c_readstr (TREE_STRING_POINTER (str) + offset, mode, false);
>  }
>
>  /* Generate code for computing expression EXP,
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-09-29  6:29 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-28 13:36 [PATCH] Simplify & expand c_readstr Richard Sandiford
2023-09-29  6:28 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).