From: Richard Biener <richard.guenther@gmail.com>
To: gcc-patches@gcc.gnu.org, HAO CHEN GUI <guihaoc@linux.ibm.com>,
"Kewen.Lin" <linkw@linux.ibm.com>,
richard.sandiford@arm.com
Subject: Re: [PATCH] Simplify & expand c_readstr
Date: Fri, 29 Sep 2023 08:28:58 +0200 [thread overview]
Message-ID: <CAFiYyc0rWjY6NGCQRYEF=7oAA5xuUiYH-uiBmdDhCwkZcvzzDA@mail.gmail.com> (raw)
In-Reply-To: <mpto7hms9lu.fsf@arm.com>
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
>
prev parent reply other threads:[~2023-09-29 6:29 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-28 13:36 Richard Sandiford
2023-09-29 6:28 ` Richard Biener [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFiYyc0rWjY6NGCQRYEF=7oAA5xuUiYH-uiBmdDhCwkZcvzzDA@mail.gmail.com' \
--to=richard.guenther@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=guihaoc@linux.ibm.com \
--cc=linkw@linux.ibm.com \
--cc=richard.sandiford@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).