public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix store-merging vuse handling (PR tree-optimization/83170, PR tree-optimization/83241)
@ 2017-12-01 22:36 Jakub Jelinek
  2017-12-02  7:55 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2017-12-01 22:36 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

Hi!

The bswap infrastructure uses the vuse field to make sure all the loads are
having the same gimple_vuse and also uses it in bswap_replace.
When this infrastructure is used inside of the store-merging pass, the
problem is that the old stores are being removed and new added, so
gimple_vuse of the loads we record during process_stmt can change.
So, this patch updates the vuse fields before we plan to use it (in
try_coalesce_bswap for the checking and in output_merged_stores for the
bswap_replace purposes).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-12-01  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/83170
	PR tree-optimization/83241
	* gimple-ssa-store-merging.c
	(imm_store_chain_info::try_coalesce_bswap): Update vuse field from
	gimple_vuse (ins_stmt) in case it has changed.
	(imm_store_chain_info::output_merged_store): Likewise.

	* gcc.dg/store_merging_17.c: New test.

--- gcc/gimple-ssa-store-merging.c.jj	2017-12-01 09:17:36.000000000 +0100
+++ gcc/gimple-ssa-store-merging.c	2017-12-01 16:03:40.806918965 +0100
@@ -2384,6 +2384,9 @@ imm_store_chain_info::try_coalesce_bswap
       this_n.type = type;
       if (!this_n.base_addr)
 	this_n.range = try_size / BITS_PER_UNIT;
+      else
+	/* Update vuse in case it has changed by output_merged_stores.  */
+	this_n.vuse = gimple_vuse (info->ins_stmt);
       unsigned int bitpos = info->bitpos - infof->bitpos;
       if (!do_shift_rotate (LSHIFT_EXPR, &this_n,
 			    BYTES_BIG_ENDIAN
@@ -3341,10 +3344,16 @@ imm_store_chain_info::output_merged_stor
 	 we've checked the aliasing already in try_coalesce_bswap and
 	 we want to sink the need load into seq.  So need to use new_vuse
 	 on the load.  */
-      if (n->base_addr && n->vuse == NULL)
+      if (n->base_addr)
 	{
-	  n->vuse = new_vuse;
-	  ins_stmt = NULL;
+	  if (n->vuse == NULL)
+	    {
+	      n->vuse = new_vuse;
+	      ins_stmt = NULL;
+	    }
+	  else
+	    /* Update vuse in case it has changed by output_merged_stores.  */
+	    n->vuse = gimple_vuse (ins_stmt);
 	}
       bswap_res = bswap_replace (gsi_start (seq), ins_stmt, fndecl,
 				 bswap_type, load_type, n, bswap);
--- gcc/testsuite/gcc.dg/store_merging_17.c.jj	2017-12-01 16:07:20.590224536 +0100
+++ gcc/testsuite/gcc.dg/store_merging_17.c	2017-12-01 16:07:01.000000000 +0100
@@ -0,0 +1,17 @@
+/* PR tree-optimization/83241 */
+/* { dg-do compile { target store_merge } } */
+/* { dg-options "-O2" } */
+
+struct S { int a; short b[32]; } e;
+struct T { volatile int c; int d; } f;
+
+void
+foo ()
+{
+  struct T g = f;
+  e.b[0] = 6;
+  e.b[1] = 6;
+  e.b[4] = g.d;
+  e.b[5] = g.d >> 16;
+  e.a = 1;
+}

	Jakub

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

* Re: [PATCH] Fix store-merging vuse handling (PR tree-optimization/83170, PR tree-optimization/83241)
  2017-12-01 22:36 [PATCH] Fix store-merging vuse handling (PR tree-optimization/83170, PR tree-optimization/83241) Jakub Jelinek
@ 2017-12-02  7:55 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2017-12-02  7:55 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On December 1, 2017 11:36:19 PM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>The bswap infrastructure uses the vuse field to make sure all the loads
>are
>having the same gimple_vuse and also uses it in bswap_replace.
>When this infrastructure is used inside of the store-merging pass, the
>problem is that the old stores are being removed and new added, so
>gimple_vuse of the loads we record during process_stmt can change.
>So, this patch updates the vuse fields before we plan to use it (in
>try_coalesce_bswap for the checking and in output_merged_stores for the
>bswap_replace purposes).
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK. 

Richard. 

>2017-12-01  Jakub Jelinek  <jakub@redhat.com>
>
>	PR tree-optimization/83170
>	PR tree-optimization/83241
>	* gimple-ssa-store-merging.c
>	(imm_store_chain_info::try_coalesce_bswap): Update vuse field from
>	gimple_vuse (ins_stmt) in case it has changed.
>	(imm_store_chain_info::output_merged_store): Likewise.
>
>	* gcc.dg/store_merging_17.c: New test.
>
>--- gcc/gimple-ssa-store-merging.c.jj	2017-12-01 09:17:36.000000000
>+0100
>+++ gcc/gimple-ssa-store-merging.c	2017-12-01 16:03:40.806918965 +0100
>@@ -2384,6 +2384,9 @@ imm_store_chain_info::try_coalesce_bswap
>       this_n.type = type;
>       if (!this_n.base_addr)
> 	this_n.range = try_size / BITS_PER_UNIT;
>+      else
>+	/* Update vuse in case it has changed by output_merged_stores.  */
>+	this_n.vuse = gimple_vuse (info->ins_stmt);
>       unsigned int bitpos = info->bitpos - infof->bitpos;
>       if (!do_shift_rotate (LSHIFT_EXPR, &this_n,
> 			    BYTES_BIG_ENDIAN
>@@ -3341,10 +3344,16 @@ imm_store_chain_info::output_merged_stor
> 	 we've checked the aliasing already in try_coalesce_bswap and
> 	 we want to sink the need load into seq.  So need to use new_vuse
> 	 on the load.  */
>-      if (n->base_addr && n->vuse == NULL)
>+      if (n->base_addr)
> 	{
>-	  n->vuse = new_vuse;
>-	  ins_stmt = NULL;
>+	  if (n->vuse == NULL)
>+	    {
>+	      n->vuse = new_vuse;
>+	      ins_stmt = NULL;
>+	    }
>+	  else
>+	    /* Update vuse in case it has changed by output_merged_stores. 
>*/
>+	    n->vuse = gimple_vuse (ins_stmt);
> 	}
>       bswap_res = bswap_replace (gsi_start (seq), ins_stmt, fndecl,
> 				 bswap_type, load_type, n, bswap);
>--- gcc/testsuite/gcc.dg/store_merging_17.c.jj	2017-12-01
>16:07:20.590224536 +0100
>+++ gcc/testsuite/gcc.dg/store_merging_17.c	2017-12-01
>16:07:01.000000000 +0100
>@@ -0,0 +1,17 @@
>+/* PR tree-optimization/83241 */
>+/* { dg-do compile { target store_merge } } */
>+/* { dg-options "-O2" } */
>+
>+struct S { int a; short b[32]; } e;
>+struct T { volatile int c; int d; } f;
>+
>+void
>+foo ()
>+{
>+  struct T g = f;
>+  e.b[0] = 6;
>+  e.b[1] = 6;
>+  e.b[4] = g.d;
>+  e.b[5] = g.d >> 16;
>+  e.a = 1;
>+}
>
>	Jakub

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

end of thread, other threads:[~2017-12-02  7:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-01 22:36 [PATCH] Fix store-merging vuse handling (PR tree-optimization/83170, PR tree-optimization/83241) Jakub Jelinek
2017-12-02  7:55 ` 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).