public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] bswap: Fix up recent vector CONSTRUCTOR optimization [PR98568]
@ 2021-01-07  8:31 Jakub Jelinek
  2021-01-07  8:40 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2021-01-07  8:31 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

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?

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


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

* Re: [PATCH] bswap: Fix up recent vector CONSTRUCTOR optimization [PR98568]
  2021-01-07  8:31 [PATCH] bswap: Fix up recent vector CONSTRUCTOR optimization [PR98568] Jakub Jelinek
@ 2021-01-07  8:40 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2021-01-07  8:40 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

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)

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

end of thread, other threads:[~2021-01-07  8:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-07  8:31 [PATCH] bswap: Fix up recent vector CONSTRUCTOR optimization [PR98568] Jakub Jelinek
2021-01-07  8:40 ` 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).