From: Richard Biener <rguenther@suse.de>
To: Jakub Jelinek <jakub@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] bswap: Fix up recent vector CONSTRUCTOR optimization [PR98568]
Date: Thu, 7 Jan 2021 09:40:54 +0100 (CET) [thread overview]
Message-ID: <nycvar.YFH.7.76.2101070940470.17979@zhemvz.fhfr.qr> (raw)
In-Reply-To: <20210107083147.GK725145@tucnak>
On Thu, 7 Jan 2021, Jakub Jelinek wrote:
> Hi!
>
> As the testcase shows, bswap can match even byte-swapping or indentity
> from low part of some wider SSA_NAME.
> For bswap replacement other than for vector CONSTRUCTOR the code has been
> using NOP_EXPR casts if the types weren't compatible, but for vectors
> we need to use VIEW_CONVERT_EXPR. The problem with the latter is that
> we require that it has the same size, which isn't guaranteed, so this patch
> in those cases first adds a narrowing NOP_EXPR cast and only afterwards
> does a VIEW_CONVERT_EXPR.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
OK.
Thanks,
Richard.
> 2021-01-06 Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/98568
> * gimple-ssa-store-merging.c (bswap_view_convert): New function.
> (bswap_replace): Use it.
>
> * g++.dg/torture/pr98568.C: New test.
>
> --- gcc/gimple-ssa-store-merging.c.jj 2021-01-05 16:15:58.965337667 +0100
> +++ gcc/gimple-ssa-store-merging.c 2021-01-06 19:20:37.225578412 +0100
> @@ -978,6 +978,25 @@ public:
>
> }; // class pass_optimize_bswap
>
> +/* Helper function for bswap_replace. Build VIEW_CONVERT_EXPR from
> + VAL to TYPE. If VAL has different type size, emit a NOP_EXPR cast
> + first. */
> +
> +static tree
> +bswap_view_convert (gimple_stmt_iterator *gsi, tree type, tree val)
> +{
> + gcc_assert (INTEGRAL_TYPE_P (TREE_TYPE (val)));
> + if (TYPE_SIZE (type) != TYPE_SIZE (TREE_TYPE (val)))
> + {
> + HOST_WIDE_INT prec = TREE_INT_CST_LOW (TYPE_SIZE (type));
> + tree itype = build_nonstandard_integer_type (prec, 1);
> + gimple *g = gimple_build_assign (make_ssa_name (itype), NOP_EXPR, val);
> + gsi_insert_before (gsi, g, GSI_SAME_STMT);
> + val = gimple_assign_lhs (g);
> + }
> + return build1 (VIEW_CONVERT_EXPR, type, val);
> +}
> +
> /* Perform the bswap optimization: replace the expression computed in the rhs
> of gsi_stmt (GSI) (or if NULL add instead of replace) by an equivalent
> bswap, load or load + bswap expression.
> @@ -1100,7 +1119,7 @@ bswap_replace (gimple_stmt_iterator gsi,
> gimple_set_vuse (load_stmt, n->vuse);
> gsi_insert_before (&gsi, load_stmt, GSI_SAME_STMT);
> if (conv_code == VIEW_CONVERT_EXPR)
> - val_tmp = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (tgt), val_tmp);
> + val_tmp = bswap_view_convert (&gsi, TREE_TYPE (tgt), val_tmp);
> gimple_assign_set_rhs_with_ops (&gsi, conv_code, val_tmp);
> update_stmt (cur_stmt);
> }
> @@ -1144,7 +1163,7 @@ bswap_replace (gimple_stmt_iterator gsi,
> if (!is_gimple_val (src))
> return NULL_TREE;
> if (conv_code == VIEW_CONVERT_EXPR)
> - src = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (tgt), src);
> + src = bswap_view_convert (&gsi, TREE_TYPE (tgt), src);
> g = gimple_build_assign (tgt, conv_code, src);
> }
> else if (cur_stmt)
> @@ -1227,7 +1246,7 @@ bswap_replace (gimple_stmt_iterator gsi,
> tmp = make_temp_ssa_name (bswap_type, NULL, "bswapdst");
> tree atmp = tmp;
> if (conv_code == VIEW_CONVERT_EXPR)
> - atmp = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (tgt), tmp);
> + atmp = bswap_view_convert (&gsi, TREE_TYPE (tgt), tmp);
> convert_stmt = gimple_build_assign (tgt, conv_code, atmp);
> gsi_insert_after (&gsi, convert_stmt, GSI_SAME_STMT);
> }
> --- gcc/testsuite/g++.dg/torture/pr98568.C.jj 2021-01-06 19:19:25.940377456 +0100
> +++ gcc/testsuite/g++.dg/torture/pr98568.C 2021-01-06 19:19:16.608482074 +0100
> @@ -0,0 +1,37 @@
> +// PR tree-optimization/98568
> +// { dg-do compile }
> +
> +char a[2];
> +char b[4];
> +
> +void
> +foo (int x)
> +{
> + a[1] = x >> 8;
> + a[0] = x;
> +}
> +
> +void
> +bar (long long x)
> +{
> + b[3] = x >> 24;
> + b[2] = x >> 16;
> + b[1] = x >> 8;
> + b[0] = x;
> +}
> +
> +void
> +baz (int x)
> +{
> + a[0] = x >> 8;
> + a[1] = x;
> +}
> +
> +void
> +qux (long long x)
> +{
> + b[0] = x >> 24;
> + b[1] = x >> 16;
> + b[2] = x >> 8;
> + b[3] = x;
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
prev parent reply other threads:[~2021-01-07 8:40 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-07 8:31 Jakub Jelinek
2021-01-07 8:40 ` 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=nycvar.YFH.7.76.2101070940470.17979@zhemvz.fhfr.qr \
--to=rguenther@suse.de \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.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).