diff mbox series

Fix tree-emutls ADDR_EXPR handling (PR middle-end/83945)

Message ID 20180119215208.GZ2063@tucnak
State New
Headers show
Series Fix tree-emutls ADDR_EXPR handling (PR middle-end/83945) | expand

Commit Message

Jakub Jelinek Jan. 19, 2018, 9:52 p.m. UTC
Hi!

Before emutls lowering, expressions like &MEM_REF[(whatever)&e].a
for static __thread e is considered gimple invariant (after all, for
native TLS we do that too) and gimple invariants are shareable trees.
lower_emutls* then has code to update the VAR_DECL in there with an SSA_NAME
and if needed, split it appart into another stmt (i.e. manually regimplify)
if needed, but doesn't take into account the possible tree sharing,
which can result in 1) invalid tree sharing, because after changing the
VAR_DECL for a SSA_NAME it is no longer gimple invariant 2) might not
break it appart anymore the second and further times 3) might use SSA_NAME
that doesn't dominate it.

Fixed by checking if we'll need to update the ADDR_EXPR operand or anything
in there and unsharing if so before actually changing it.

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

2018-01-19  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/83945
	* tree-emutls.c: Include gimplify.h.
	(lower_emutls_2): New function.
	(lower_emutls_1): If ADDR_EXPR is a gimple invariant and walk_tree
	with lower_emutls_2 callback finds some TLS decl in it, unshare_expr
	it before further processing.

	* gcc.dg/tls/pr83945.c: New test.


	Jakub

Comments

Richard Biener Jan. 20, 2018, 7:03 a.m. UTC | #1
On January 19, 2018 10:52:08 PM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>Before emutls lowering, expressions like &MEM_REF[(whatever)&e].a
>for static __thread e is considered gimple invariant (after all, for
>native TLS we do that too) and gimple invariants are shareable trees.
>lower_emutls* then has code to update the VAR_DECL in there with an
>SSA_NAME
>and if needed, split it appart into another stmt (i.e. manually
>regimplify)
>if needed, but doesn't take into account the possible tree sharing,
>which can result in 1) invalid tree sharing, because after changing the
>VAR_DECL for a SSA_NAME it is no longer gimple invariant 2) might not
>break it appart anymore the second and further times 3) might use
>SSA_NAME
>that doesn't dominate it.
>
>Fixed by checking if we'll need to update the ADDR_EXPR operand or
>anything
>in there and unsharing if so before actually changing it.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK. 

Richard. 

>2018-01-19  Jakub Jelinek  <jakub@redhat.com>
>
>	PR middle-end/83945
>	* tree-emutls.c: Include gimplify.h.
>	(lower_emutls_2): New function.
>	(lower_emutls_1): If ADDR_EXPR is a gimple invariant and walk_tree
>	with lower_emutls_2 callback finds some TLS decl in it, unshare_expr
>	it before further processing.
>
>	* gcc.dg/tls/pr83945.c: New test.
>
>--- gcc/tree-emutls.c.jj	2018-01-03 10:19:54.790533899 +0100
>+++ gcc/tree-emutls.c	2018-01-19 19:11:14.849321308 +0100
>@@ -34,6 +34,7 @@ along with GCC; see the file COPYING3.
> #include "gimple-walk.h"
> #include "langhooks.h"
> #include "tree-iterator.h"
>+#include "gimplify.h"
> 
>/* Whenever a target does not support thread-local storage (TLS)
>natively,
>  we can emulate it with some run-time support in libgcc.  This will in
>@@ -429,6 +430,20 @@ gen_emutls_addr (tree decl, struct lower
>   return addr;
> }
> 
>+/* Callback for lower_emutls_1, return non-NULL if there is any TLS
>+   VAR_DECL in the subexpressions.  */
>+
>+static tree
>+lower_emutls_2 (tree *ptr, int *walk_subtrees, void *)
>+{
>+  tree t = *ptr;
>+  if (TREE_CODE (t) == VAR_DECL)
>+    return DECL_THREAD_LOCAL_P (t) ? t : NULL_TREE;
>+  else if (!EXPR_P (t))
>+    *walk_subtrees = 0;
>+  return NULL_TREE;
>+}
>+
>/* Callback for walk_gimple_op.  D = WI->INFO is a struct
>lower_emutls_data.
>  Given an operand *PTR within D->STMT, if the operand references a TLS
>   variable, then lower the reference to a call to the runtime.  Insert
>@@ -455,6 +470,13 @@ lower_emutls_1 (tree *ptr, int *walk_sub
> 	{
> 	  bool save_changed;
> 
>+	  /* Gimple invariants are shareable trees, so before changing
>+	     anything in them if we will need to change anything, unshare
>+	     them.  */
>+	  if (is_gimple_min_invariant (t)
>+	      && walk_tree (&TREE_OPERAND (t, 0), lower_emutls_2, NULL,
>NULL))
>+	    *ptr = t = unshare_expr (t);
>+
> 	  /* If we're allowed more than just is_gimple_val, continue.  */
> 	  if (!wi->val_only)
> 	    {
>--- gcc/testsuite/gcc.dg/tls/pr83945.c.jj	2018-01-19 19:16:52.376346273
>+0100
>+++ gcc/testsuite/gcc.dg/tls/pr83945.c	2018-01-19 19:16:25.186344529
>+0100
>@@ -0,0 +1,21 @@
>+/* PR middle-end/83945 */
>+/* { dg-do compile { target tls } } */
>+/* { dg-options "-O2" } */
>+
>+struct S { int a[1]; };
>+__thread struct T { int c; } e;
>+int f;
>+void bar (int);
>+
>+void
>+foo (int f, int x)
>+{
>+  struct S *h = (struct S *) &e.c;
>+  for (;;)
>+    {
>+      int *a = h->a, i;
>+      for (i = x; i; i--)
>+	bar (a[f]);
>+      bar (a[f]);
>+    }
>+}
>
>	Jakub
diff mbox series

Patch

--- gcc/tree-emutls.c.jj	2018-01-03 10:19:54.790533899 +0100
+++ gcc/tree-emutls.c	2018-01-19 19:11:14.849321308 +0100
@@ -34,6 +34,7 @@  along with GCC; see the file COPYING3.
 #include "gimple-walk.h"
 #include "langhooks.h"
 #include "tree-iterator.h"
+#include "gimplify.h"
 
 /* Whenever a target does not support thread-local storage (TLS) natively,
    we can emulate it with some run-time support in libgcc.  This will in
@@ -429,6 +430,20 @@  gen_emutls_addr (tree decl, struct lower
   return addr;
 }
 
+/* Callback for lower_emutls_1, return non-NULL if there is any TLS
+   VAR_DECL in the subexpressions.  */
+
+static tree
+lower_emutls_2 (tree *ptr, int *walk_subtrees, void *)
+{
+  tree t = *ptr;
+  if (TREE_CODE (t) == VAR_DECL)
+    return DECL_THREAD_LOCAL_P (t) ? t : NULL_TREE;
+  else if (!EXPR_P (t))
+    *walk_subtrees = 0;
+  return NULL_TREE;
+}
+
 /* Callback for walk_gimple_op.  D = WI->INFO is a struct lower_emutls_data.
    Given an operand *PTR within D->STMT, if the operand references a TLS
    variable, then lower the reference to a call to the runtime.  Insert
@@ -455,6 +470,13 @@  lower_emutls_1 (tree *ptr, int *walk_sub
 	{
 	  bool save_changed;
 
+	  /* Gimple invariants are shareable trees, so before changing
+	     anything in them if we will need to change anything, unshare
+	     them.  */
+	  if (is_gimple_min_invariant (t)
+	      && walk_tree (&TREE_OPERAND (t, 0), lower_emutls_2, NULL, NULL))
+	    *ptr = t = unshare_expr (t);
+
 	  /* If we're allowed more than just is_gimple_val, continue.  */
 	  if (!wi->val_only)
 	    {
--- gcc/testsuite/gcc.dg/tls/pr83945.c.jj	2018-01-19 19:16:52.376346273 +0100
+++ gcc/testsuite/gcc.dg/tls/pr83945.c	2018-01-19 19:16:25.186344529 +0100
@@ -0,0 +1,21 @@ 
+/* PR middle-end/83945 */
+/* { dg-do compile { target tls } } */
+/* { dg-options "-O2" } */
+
+struct S { int a[1]; };
+__thread struct T { int c; } e;
+int f;
+void bar (int);
+
+void
+foo (int f, int x)
+{
+  struct S *h = (struct S *) &e.c;
+  for (;;)
+    {
+      int *a = h->a, i;
+      for (i = x; i; i--)
+	bar (a[f]);
+      bar (a[f]);
+    }
+}