On January 19, 2018 10:52:08 PM GMT+01:00, Jakub Jelinek <ja...@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 <ja...@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