[v2] ipa-visibility: Optimize TLS access [PR99619]

  From: Artem Klimov <jakmobius@gmail.com>

Fix PR99619, which asks to optimize TLS model based on visibility.
The fix is implemented as an IPA optimization: this allows to take
optimized visibility status into account (as well as avoid modifying
all language frontends).

2022-04-17  Artem Klimov  <jakmobius@gmail.com>

gcc/ChangeLog:

	* ipa-visibility.cc (function_and_variable_visibility): Promote
	TLS access model afer visibility optimizations.
	* varasm.cc (have_optimized_refs): New helper.
	(optimize_dyn_tls_for_decl_p): New helper. Use it ...
	(decl_default_tls_model): ... here in place of 'optimize' check.

gcc/testsuite/ChangeLog:

	* gcc.dg/tls/vis-attr-gd.c: New test.
	* gcc.dg/tls/vis-attr-hidden-gd.c: New test.
	* gcc.dg/tls/vis-attr-hidden.c: New test.
	* gcc.dg/tls/vis-flag-hidden-gd.c: New test.
	* gcc.dg/tls/vis-flag-hidden.c: New test.
	* gcc.dg/tls/vis-pragma-hidden-gd.c: New test.
	* gcc.dg/tls/vis-pragma-hidden.c: New test.

Co-Authored-By:  Alexander Monakov  <amonakov@gcc.gnu.org>
Signed-off-by: Artem Klimov <jakmobius@gmail.com>
---

v2: run the new loop in ipa-visibility only in the whole-program IPA pass;
    in decl_default_tls_model, check if any referring function is optimized
    when 'optimize == 0' (when running in LTO mode)


Note for reviewers: I noticed there's a place which tries to avoid TLS
promotion, but the comment seems wrong and I could not find a testcase.
I'd suggest we remove it. The compiler can only promote general-dynamic
to local-dynamic and initial-exec to local-exec. The comment refers to
promoting x-dynamic to y-exec, but that cannot happen AFAICT:
https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=8e1ba78f1b8eedd6c65c6f0e6d6d09a801de5d3d


 gcc/ipa-visibility.cc                         | 19 +++++++++++
 gcc/testsuite/gcc.dg/tls/vis-attr-gd.c        | 12 +++++++
 gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c | 13 ++++++++
 gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c    | 12 +++++++
 gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c | 13 ++++++++
 gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c    | 12 +++++++
 .../gcc.dg/tls/vis-pragma-hidden-gd.c         | 17 ++++++++++
 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c  | 16 ++++++++++
 gcc/varasm.cc                                 | 32 ++++++++++++++++++-
 9 files changed, 145 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
 create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
 create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
 create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
 create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
 create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
 create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
  

Ping.

On Thu, 7 Jul 2022, Alexander Monakov via Gcc-patches wrote:

> From: Artem Klimov <jakmobius@gmail.com>
> 
> Fix PR99619, which asks to optimize TLS model based on visibility.
> The fix is implemented as an IPA optimization: this allows to take
> optimized visibility status into account (as well as avoid modifying
> all language frontends).
> 
> 2022-04-17  Artem Klimov  <jakmobius@gmail.com>
> 
> gcc/ChangeLog:
> 
> 	* ipa-visibility.cc (function_and_variable_visibility): Promote
> 	TLS access model afer visibility optimizations.
> 	* varasm.cc (have_optimized_refs): New helper.
> 	(optimize_dyn_tls_for_decl_p): New helper. Use it ...
> 	(decl_default_tls_model): ... here in place of 'optimize' check.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/tls/vis-attr-gd.c: New test.
> 	* gcc.dg/tls/vis-attr-hidden-gd.c: New test.
> 	* gcc.dg/tls/vis-attr-hidden.c: New test.
> 	* gcc.dg/tls/vis-flag-hidden-gd.c: New test.
> 	* gcc.dg/tls/vis-flag-hidden.c: New test.
> 	* gcc.dg/tls/vis-pragma-hidden-gd.c: New test.
> 	* gcc.dg/tls/vis-pragma-hidden.c: New test.
> 
> Co-Authored-By:  Alexander Monakov  <amonakov@gcc.gnu.org>
> Signed-off-by: Artem Klimov <jakmobius@gmail.com>
> ---
> 
> v2: run the new loop in ipa-visibility only in the whole-program IPA pass;
>     in decl_default_tls_model, check if any referring function is optimized
>     when 'optimize == 0' (when running in LTO mode)
> 
> 
> Note for reviewers: I noticed there's a place which tries to avoid TLS
> promotion, but the comment seems wrong and I could not find a testcase.
> I'd suggest we remove it. The compiler can only promote general-dynamic
> to local-dynamic and initial-exec to local-exec. The comment refers to
> promoting x-dynamic to y-exec, but that cannot happen AFAICT:
> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=8e1ba78f1b8eedd6c65c6f0e6d6d09a801de5d3d
> 
> 
>  gcc/ipa-visibility.cc                         | 19 +++++++++++
>  gcc/testsuite/gcc.dg/tls/vis-attr-gd.c        | 12 +++++++
>  gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c | 13 ++++++++
>  gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c    | 12 +++++++
>  gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c | 13 ++++++++
>  gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c    | 12 +++++++
>  .../gcc.dg/tls/vis-pragma-hidden-gd.c         | 17 ++++++++++
>  gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c  | 16 ++++++++++
>  gcc/varasm.cc                                 | 32 ++++++++++++++++++-
>  9 files changed, 145 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
> 
> diff --git a/gcc/ipa-visibility.cc b/gcc/ipa-visibility.cc
> index 8a27e7bcd..3ed2b7cf6 100644
> --- a/gcc/ipa-visibility.cc
> +++ b/gcc/ipa-visibility.cc
> @@ -873,6 +873,25 @@ function_and_variable_visibility (bool whole_program)
>  	}
>      }
>  
> +  if (symtab->state >= IPA_SSA)
> +    {
> +      FOR_EACH_VARIABLE (vnode)
> +	{
> +	  tree decl = vnode->decl;
> +
> +	  /* Upgrade TLS access model based on optimized visibility status,
> +	     unless it was specified explicitly or no references remain.  */
> +	  if (DECL_THREAD_LOCAL_P (decl)
> +	      && !lookup_attribute ("tls_model", DECL_ATTRIBUTES (decl))
> +	      && vnode->ref_list.referring.length ())
> +	    {
> +	      enum tls_model new_model = decl_default_tls_model (decl);
> +	      gcc_checking_assert (new_model >= decl_tls_model (decl));
> +	      set_decl_tls_model (decl, new_model);
> +	    }
> +	}
> +    }
> +
>    if (dump_file)
>      {
>        fprintf (dump_file, "\nMarking local functions:");
> diff --git a/gcc/varasm.cc b/gcc/varasm.cc
> index 4db8506b1..de149e82c 100644
> --- a/gcc/varasm.cc
> +++ b/gcc/varasm.cc
> @@ -6679,6 +6679,36 @@ init_varasm_once (void)
>  #endif
>  }
>  
> +/* Determine whether SYMBOL is used in any optimized function.  */
> +
> +static bool
> +have_optimized_refs (struct symtab_node *symbol)
> +{
> +  struct ipa_ref *ref;
> +
> +  for (int i = 0; symbol->iterate_referring (i, ref); i++)
> +    {
> +      cgraph_node *cnode = dyn_cast <cgraph_node *> (ref->referring);
> +
> +      if (cnode && opt_for_fn (cnode->decl, optimize))
> +	return true;
> +    }
> +
> +  return false;
> +}
> +
> +/* Check if promoting general-dynamic TLS access model to local-dynamic is
> +   desirable for DECL.  */
> +
> +static bool
> +optimize_dyn_tls_for_decl_p (const_tree decl)
> +{
> +  if (optimize)
> +    return true;
> +  return symtab->state >= IPA && have_optimized_refs (symtab_node::get (decl));
> +}
> +
> +
>  enum tls_model
>  decl_default_tls_model (const_tree decl)
>  {
> @@ -6696,7 +6726,7 @@ decl_default_tls_model (const_tree decl)
>  
>    /* Local dynamic is inefficient when we're not combining the
>       parts of the address.  */
> -  else if (optimize && is_local)
> +  else if (is_local && optimize_dyn_tls_for_decl_p (decl))
>      kind = TLS_MODEL_LOCAL_DYNAMIC;
>    else
>      kind = TLS_MODEL_GLOBAL_DYNAMIC;
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c b/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
> new file mode 100644
> index 000000000..89a248a80
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> +
> +// tls_model should be global-dynamic due to explicitly specified attribute
> +__attribute__((tls_model("global-dynamic")))
> +__thread int x;
> +
> +void reference() { x++; }
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
> new file mode 100644
> index 000000000..e32565588
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> +
> +// tls_model should be global-dynamic due to explicitly specified attribute
> +__attribute__((visibility("hidden")))
> +__attribute__((tls_model("global-dynamic")))
> +__thread int x;
> +
> +void reference() { x++; }
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
> new file mode 100644
> index 000000000..0d43fc565
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> +
> +//tls_model should be local-dynamic due to visibility("hidden")
> +__attribute__((visibility("hidden")))
> +__thread int x;
> +
> +void reference() { x++; }
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-local-dynamic" "whole-program" } } */
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
> new file mode 100644
> index 000000000..cad41e0c8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program -fvisibility=hidden" } */
> +
> +
> +// tls_model should be global-dynamic due to explicitly specified attribute
> +__attribute__((tls_model("global-dynamic")))
> +__thread int x;
> +
> +void reference() { x++; }
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
> new file mode 100644
> index 000000000..a15df092d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program -fvisibility=hidden" } */
> +
> +
> +// tls_model should be local-dynamic due to -fvisibility=hidden
> +__thread int x;
> +
> +void reference() { x++; }
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-local-dynamic" "whole-program" } } */
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
> new file mode 100644
> index 000000000..3b3598134
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> +
> +
> +#pragma GCC visibility push(hidden)
> +
> +// tls_model should be global-dynamic due to explicitly specified attribute
> +__attribute__((tls_model("global-dynamic")))
> +__thread int x;
> +
> +#pragma GCC visibility pop
> +
> +void reference() { x++; }
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
> new file mode 100644
> index 000000000..1be976442
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> +
> +
> +#pragma GCC visibility push(hidden)
> +
> +// tls_model should be local-dynamic due to a pragma
> +__thread int x;
> +
> +#pragma GCC visibility pop
> +
> +void reference() { x++; }
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-local-dynamic" "whole-program" } } */
>
  

Ping^2.

On Wed, 20 Jul 2022, Alexander Monakov wrote:

> 
> Ping.
> 
> On Thu, 7 Jul 2022, Alexander Monakov via Gcc-patches wrote:
> 
> > From: Artem Klimov <jakmobius@gmail.com>
> > 
> > Fix PR99619, which asks to optimize TLS model based on visibility.
> > The fix is implemented as an IPA optimization: this allows to take
> > optimized visibility status into account (as well as avoid modifying
> > all language frontends).
> > 
> > 2022-04-17  Artem Klimov  <jakmobius@gmail.com>
> > 
> > gcc/ChangeLog:
> > 
> > 	* ipa-visibility.cc (function_and_variable_visibility): Promote
> > 	TLS access model afer visibility optimizations.
> > 	* varasm.cc (have_optimized_refs): New helper.
> > 	(optimize_dyn_tls_for_decl_p): New helper. Use it ...
> > 	(decl_default_tls_model): ... here in place of 'optimize' check.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* gcc.dg/tls/vis-attr-gd.c: New test.
> > 	* gcc.dg/tls/vis-attr-hidden-gd.c: New test.
> > 	* gcc.dg/tls/vis-attr-hidden.c: New test.
> > 	* gcc.dg/tls/vis-flag-hidden-gd.c: New test.
> > 	* gcc.dg/tls/vis-flag-hidden.c: New test.
> > 	* gcc.dg/tls/vis-pragma-hidden-gd.c: New test.
> > 	* gcc.dg/tls/vis-pragma-hidden.c: New test.
> > 
> > Co-Authored-By:  Alexander Monakov  <amonakov@gcc.gnu.org>
> > Signed-off-by: Artem Klimov <jakmobius@gmail.com>
> > ---
> > 
> > v2: run the new loop in ipa-visibility only in the whole-program IPA pass;
> >     in decl_default_tls_model, check if any referring function is optimized
> >     when 'optimize == 0' (when running in LTO mode)
> > 
> > 
> > Note for reviewers: I noticed there's a place which tries to avoid TLS
> > promotion, but the comment seems wrong and I could not find a testcase.
> > I'd suggest we remove it. The compiler can only promote general-dynamic
> > to local-dynamic and initial-exec to local-exec. The comment refers to
> > promoting x-dynamic to y-exec, but that cannot happen AFAICT:
> > https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=8e1ba78f1b8eedd6c65c6f0e6d6d09a801de5d3d
> > 
> > 
> >  gcc/ipa-visibility.cc                         | 19 +++++++++++
> >  gcc/testsuite/gcc.dg/tls/vis-attr-gd.c        | 12 +++++++
> >  gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c | 13 ++++++++
> >  gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c    | 12 +++++++
> >  gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c | 13 ++++++++
> >  gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c    | 12 +++++++
> >  .../gcc.dg/tls/vis-pragma-hidden-gd.c         | 17 ++++++++++
> >  gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c  | 16 ++++++++++
> >  gcc/varasm.cc                                 | 32 ++++++++++++++++++-
> >  9 files changed, 145 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
> >  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
> >  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
> >  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
> >  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
> >  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
> >  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
> > 
> > diff --git a/gcc/ipa-visibility.cc b/gcc/ipa-visibility.cc
> > index 8a27e7bcd..3ed2b7cf6 100644
> > --- a/gcc/ipa-visibility.cc
> > +++ b/gcc/ipa-visibility.cc
> > @@ -873,6 +873,25 @@ function_and_variable_visibility (bool whole_program)
> >  	}
> >      }
> >  
> > +  if (symtab->state >= IPA_SSA)
> > +    {
> > +      FOR_EACH_VARIABLE (vnode)
> > +	{
> > +	  tree decl = vnode->decl;
> > +
> > +	  /* Upgrade TLS access model based on optimized visibility status,
> > +	     unless it was specified explicitly or no references remain.  */
> > +	  if (DECL_THREAD_LOCAL_P (decl)
> > +	      && !lookup_attribute ("tls_model", DECL_ATTRIBUTES (decl))
> > +	      && vnode->ref_list.referring.length ())
> > +	    {
> > +	      enum tls_model new_model = decl_default_tls_model (decl);
> > +	      gcc_checking_assert (new_model >= decl_tls_model (decl));
> > +	      set_decl_tls_model (decl, new_model);
> > +	    }
> > +	}
> > +    }
> > +
> >    if (dump_file)
> >      {
> >        fprintf (dump_file, "\nMarking local functions:");
> > diff --git a/gcc/varasm.cc b/gcc/varasm.cc
> > index 4db8506b1..de149e82c 100644
> > --- a/gcc/varasm.cc
> > +++ b/gcc/varasm.cc
> > @@ -6679,6 +6679,36 @@ init_varasm_once (void)
> >  #endif
> >  }
> >  
> > +/* Determine whether SYMBOL is used in any optimized function.  */
> > +
> > +static bool
> > +have_optimized_refs (struct symtab_node *symbol)
> > +{
> > +  struct ipa_ref *ref;
> > +
> > +  for (int i = 0; symbol->iterate_referring (i, ref); i++)
> > +    {
> > +      cgraph_node *cnode = dyn_cast <cgraph_node *> (ref->referring);
> > +
> > +      if (cnode && opt_for_fn (cnode->decl, optimize))
> > +	return true;
> > +    }
> > +
> > +  return false;
> > +}
> > +
> > +/* Check if promoting general-dynamic TLS access model to local-dynamic is
> > +   desirable for DECL.  */
> > +
> > +static bool
> > +optimize_dyn_tls_for_decl_p (const_tree decl)
> > +{
> > +  if (optimize)
> > +    return true;
> > +  return symtab->state >= IPA && have_optimized_refs (symtab_node::get (decl));
> > +}
> > +
> > +
> >  enum tls_model
> >  decl_default_tls_model (const_tree decl)
> >  {
> > @@ -6696,7 +6726,7 @@ decl_default_tls_model (const_tree decl)
> >  
> >    /* Local dynamic is inefficient when we're not combining the
> >       parts of the address.  */
> > -  else if (optimize && is_local)
> > +  else if (is_local && optimize_dyn_tls_for_decl_p (decl))
> >      kind = TLS_MODEL_LOCAL_DYNAMIC;
> >    else
> >      kind = TLS_MODEL_GLOBAL_DYNAMIC;
> > diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c b/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
> > new file mode 100644
> > index 000000000..89a248a80
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
> > @@ -0,0 +1,12 @@
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target fpic } */
> > +/* { dg-require-effective-target tls } */
> > +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> > +
> > +// tls_model should be global-dynamic due to explicitly specified attribute
> > +__attribute__((tls_model("global-dynamic")))
> > +__thread int x;
> > +
> > +void reference() { x++; }
> > +
> > +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> > diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
> > new file mode 100644
> > index 000000000..e32565588
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
> > @@ -0,0 +1,13 @@
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target fpic } */
> > +/* { dg-require-effective-target tls } */
> > +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> > +
> > +// tls_model should be global-dynamic due to explicitly specified attribute
> > +__attribute__((visibility("hidden")))
> > +__attribute__((tls_model("global-dynamic")))
> > +__thread int x;
> > +
> > +void reference() { x++; }
> > +
> > +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> > diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
> > new file mode 100644
> > index 000000000..0d43fc565
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
> > @@ -0,0 +1,12 @@
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target fpic } */
> > +/* { dg-require-effective-target tls } */
> > +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> > +
> > +//tls_model should be local-dynamic due to visibility("hidden")
> > +__attribute__((visibility("hidden")))
> > +__thread int x;
> > +
> > +void reference() { x++; }
> > +
> > +/* { dg-final { scan-ipa-dump "Varpool flags: tls-local-dynamic" "whole-program" } } */
> > diff --git a/gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
> > new file mode 100644
> > index 000000000..cad41e0c8
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
> > @@ -0,0 +1,13 @@
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target fpic } */
> > +/* { dg-require-effective-target tls } */
> > +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program -fvisibility=hidden" } */
> > +
> > +
> > +// tls_model should be global-dynamic due to explicitly specified attribute
> > +__attribute__((tls_model("global-dynamic")))
> > +__thread int x;
> > +
> > +void reference() { x++; }
> > +
> > +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> > diff --git a/gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
> > new file mode 100644
> > index 000000000..a15df092d
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
> > @@ -0,0 +1,12 @@
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target fpic } */
> > +/* { dg-require-effective-target tls } */
> > +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program -fvisibility=hidden" } */
> > +
> > +
> > +// tls_model should be local-dynamic due to -fvisibility=hidden
> > +__thread int x;
> > +
> > +void reference() { x++; }
> > +
> > +/* { dg-final { scan-ipa-dump "Varpool flags: tls-local-dynamic" "whole-program" } } */
> > diff --git a/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
> > new file mode 100644
> > index 000000000..3b3598134
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
> > @@ -0,0 +1,17 @@
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target fpic } */
> > +/* { dg-require-effective-target tls } */
> > +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> > +
> > +
> > +#pragma GCC visibility push(hidden)
> > +
> > +// tls_model should be global-dynamic due to explicitly specified attribute
> > +__attribute__((tls_model("global-dynamic")))
> > +__thread int x;
> > +
> > +#pragma GCC visibility pop
> > +
> > +void reference() { x++; }
> > +
> > +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> > diff --git a/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
> > new file mode 100644
> > index 000000000..1be976442
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
> > @@ -0,0 +1,16 @@
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target fpic } */
> > +/* { dg-require-effective-target tls } */
> > +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> > +
> > +
> > +#pragma GCC visibility push(hidden)
> > +
> > +// tls_model should be local-dynamic due to a pragma
> > +__thread int x;
> > +
> > +#pragma GCC visibility pop
> > +
> > +void reference() { x++; }
> > +
> > +/* { dg-final { scan-ipa-dump "Varpool flags: tls-local-dynamic" "whole-program" } } */
> > 
>
  

Ping^3.

On Fri, 5 Aug 2022, Alexander Monakov wrote:

> Ping^2.
> 
> On Wed, 20 Jul 2022, Alexander Monakov wrote:
> 
> > 
> > Ping.
> > 
> > On Thu, 7 Jul 2022, Alexander Monakov via Gcc-patches wrote:
> > 
> > > From: Artem Klimov <jakmobius@gmail.com>
> > > 
> > > Fix PR99619, which asks to optimize TLS model based on visibility.
> > > The fix is implemented as an IPA optimization: this allows to take
> > > optimized visibility status into account (as well as avoid modifying
> > > all language frontends).
> > > 
> > > 2022-04-17  Artem Klimov  <jakmobius@gmail.com>
> > > 
> > > gcc/ChangeLog:
> > > 
> > > 	* ipa-visibility.cc (function_and_variable_visibility): Promote
> > > 	TLS access model afer visibility optimizations.
> > > 	* varasm.cc (have_optimized_refs): New helper.
> > > 	(optimize_dyn_tls_for_decl_p): New helper. Use it ...
> > > 	(decl_default_tls_model): ... here in place of 'optimize' check.
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > > 	* gcc.dg/tls/vis-attr-gd.c: New test.
> > > 	* gcc.dg/tls/vis-attr-hidden-gd.c: New test.
> > > 	* gcc.dg/tls/vis-attr-hidden.c: New test.
> > > 	* gcc.dg/tls/vis-flag-hidden-gd.c: New test.
> > > 	* gcc.dg/tls/vis-flag-hidden.c: New test.
> > > 	* gcc.dg/tls/vis-pragma-hidden-gd.c: New test.
> > > 	* gcc.dg/tls/vis-pragma-hidden.c: New test.
> > > 
> > > Co-Authored-By:  Alexander Monakov  <amonakov@gcc.gnu.org>
> > > Signed-off-by: Artem Klimov <jakmobius@gmail.com>
> > > ---
> > > 
> > > v2: run the new loop in ipa-visibility only in the whole-program IPA pass;
> > >     in decl_default_tls_model, check if any referring function is optimized
> > >     when 'optimize == 0' (when running in LTO mode)
> > > 
> > > 
> > > Note for reviewers: I noticed there's a place which tries to avoid TLS
> > > promotion, but the comment seems wrong and I could not find a testcase.
> > > I'd suggest we remove it. The compiler can only promote general-dynamic
> > > to local-dynamic and initial-exec to local-exec. The comment refers to
> > > promoting x-dynamic to y-exec, but that cannot happen AFAICT:
> > > https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=8e1ba78f1b8eedd6c65c6f0e6d6d09a801de5d3d
> > > 
> > > 
> > >  gcc/ipa-visibility.cc                         | 19 +++++++++++
> > >  gcc/testsuite/gcc.dg/tls/vis-attr-gd.c        | 12 +++++++
> > >  gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c | 13 ++++++++
> > >  gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c    | 12 +++++++
> > >  gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c | 13 ++++++++
> > >  gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c    | 12 +++++++
> > >  .../gcc.dg/tls/vis-pragma-hidden-gd.c         | 17 ++++++++++
> > >  gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c  | 16 ++++++++++
> > >  gcc/varasm.cc                                 | 32 ++++++++++++++++++-
> > >  9 files changed, 145 insertions(+), 1 deletion(-)
> > >  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
> > >  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
> > >  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
> > >  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
> > >  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
> > >  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
> > >  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
> > > 
> > > diff --git a/gcc/ipa-visibility.cc b/gcc/ipa-visibility.cc
> > > index 8a27e7bcd..3ed2b7cf6 100644
> > > --- a/gcc/ipa-visibility.cc
> > > +++ b/gcc/ipa-visibility.cc
> > > @@ -873,6 +873,25 @@ function_and_variable_visibility (bool whole_program)
> > >  	}
> > >      }
> > >  
> > > +  if (symtab->state >= IPA_SSA)
> > > +    {
> > > +      FOR_EACH_VARIABLE (vnode)
> > > +	{
> > > +	  tree decl = vnode->decl;
> > > +
> > > +	  /* Upgrade TLS access model based on optimized visibility status,
> > > +	     unless it was specified explicitly or no references remain.  */
> > > +	  if (DECL_THREAD_LOCAL_P (decl)
> > > +	      && !lookup_attribute ("tls_model", DECL_ATTRIBUTES (decl))
> > > +	      && vnode->ref_list.referring.length ())
> > > +	    {
> > > +	      enum tls_model new_model = decl_default_tls_model (decl);
> > > +	      gcc_checking_assert (new_model >= decl_tls_model (decl));
> > > +	      set_decl_tls_model (decl, new_model);
> > > +	    }
> > > +	}
> > > +    }
> > > +
> > >    if (dump_file)
> > >      {
> > >        fprintf (dump_file, "\nMarking local functions:");
> > > diff --git a/gcc/varasm.cc b/gcc/varasm.cc
> > > index 4db8506b1..de149e82c 100644
> > > --- a/gcc/varasm.cc
> > > +++ b/gcc/varasm.cc
> > > @@ -6679,6 +6679,36 @@ init_varasm_once (void)
> > >  #endif
> > >  }
> > >  
> > > +/* Determine whether SYMBOL is used in any optimized function.  */
> > > +
> > > +static bool
> > > +have_optimized_refs (struct symtab_node *symbol)
> > > +{
> > > +  struct ipa_ref *ref;
> > > +
> > > +  for (int i = 0; symbol->iterate_referring (i, ref); i++)
> > > +    {
> > > +      cgraph_node *cnode = dyn_cast <cgraph_node *> (ref->referring);
> > > +
> > > +      if (cnode && opt_for_fn (cnode->decl, optimize))
> > > +	return true;
> > > +    }
> > > +
> > > +  return false;
> > > +}
> > > +
> > > +/* Check if promoting general-dynamic TLS access model to local-dynamic is
> > > +   desirable for DECL.  */
> > > +
> > > +static bool
> > > +optimize_dyn_tls_for_decl_p (const_tree decl)
> > > +{
> > > +  if (optimize)
> > > +    return true;
> > > +  return symtab->state >= IPA && have_optimized_refs (symtab_node::get (decl));
> > > +}
> > > +
> > > +
> > >  enum tls_model
> > >  decl_default_tls_model (const_tree decl)
> > >  {
> > > @@ -6696,7 +6726,7 @@ decl_default_tls_model (const_tree decl)
> > >  
> > >    /* Local dynamic is inefficient when we're not combining the
> > >       parts of the address.  */
> > > -  else if (optimize && is_local)
> > > +  else if (is_local && optimize_dyn_tls_for_decl_p (decl))
> > >      kind = TLS_MODEL_LOCAL_DYNAMIC;
> > >    else
> > >      kind = TLS_MODEL_GLOBAL_DYNAMIC;
> > > diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c b/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
> > > new file mode 100644
> > > index 000000000..89a248a80
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
> > > @@ -0,0 +1,12 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-require-effective-target fpic } */
> > > +/* { dg-require-effective-target tls } */
> > > +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> > > +
> > > +// tls_model should be global-dynamic due to explicitly specified attribute
> > > +__attribute__((tls_model("global-dynamic")))
> > > +__thread int x;
> > > +
> > > +void reference() { x++; }
> > > +
> > > +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> > > diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
> > > new file mode 100644
> > > index 000000000..e32565588
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
> > > @@ -0,0 +1,13 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-require-effective-target fpic } */
> > > +/* { dg-require-effective-target tls } */
> > > +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> > > +
> > > +// tls_model should be global-dynamic due to explicitly specified attribute
> > > +__attribute__((visibility("hidden")))
> > > +__attribute__((tls_model("global-dynamic")))
> > > +__thread int x;
> > > +
> > > +void reference() { x++; }
> > > +
> > > +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> > > diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
> > > new file mode 100644
> > > index 000000000..0d43fc565
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
> > > @@ -0,0 +1,12 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-require-effective-target fpic } */
> > > +/* { dg-require-effective-target tls } */
> > > +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> > > +
> > > +//tls_model should be local-dynamic due to visibility("hidden")
> > > +__attribute__((visibility("hidden")))
> > > +__thread int x;
> > > +
> > > +void reference() { x++; }
> > > +
> > > +/* { dg-final { scan-ipa-dump "Varpool flags: tls-local-dynamic" "whole-program" } } */
> > > diff --git a/gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
> > > new file mode 100644
> > > index 000000000..cad41e0c8
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
> > > @@ -0,0 +1,13 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-require-effective-target fpic } */
> > > +/* { dg-require-effective-target tls } */
> > > +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program -fvisibility=hidden" } */
> > > +
> > > +
> > > +// tls_model should be global-dynamic due to explicitly specified attribute
> > > +__attribute__((tls_model("global-dynamic")))
> > > +__thread int x;
> > > +
> > > +void reference() { x++; }
> > > +
> > > +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> > > diff --git a/gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
> > > new file mode 100644
> > > index 000000000..a15df092d
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
> > > @@ -0,0 +1,12 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-require-effective-target fpic } */
> > > +/* { dg-require-effective-target tls } */
> > > +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program -fvisibility=hidden" } */
> > > +
> > > +
> > > +// tls_model should be local-dynamic due to -fvisibility=hidden
> > > +__thread int x;
> > > +
> > > +void reference() { x++; }
> > > +
> > > +/* { dg-final { scan-ipa-dump "Varpool flags: tls-local-dynamic" "whole-program" } } */
> > > diff --git a/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
> > > new file mode 100644
> > > index 000000000..3b3598134
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
> > > @@ -0,0 +1,17 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-require-effective-target fpic } */
> > > +/* { dg-require-effective-target tls } */
> > > +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> > > +
> > > +
> > > +#pragma GCC visibility push(hidden)
> > > +
> > > +// tls_model should be global-dynamic due to explicitly specified attribute
> > > +__attribute__((tls_model("global-dynamic")))
> > > +__thread int x;
> > > +
> > > +#pragma GCC visibility pop
> > > +
> > > +void reference() { x++; }
> > > +
> > > +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> > > diff --git a/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
> > > new file mode 100644
> > > index 000000000..1be976442
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
> > > @@ -0,0 +1,16 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-require-effective-target fpic } */
> > > +/* { dg-require-effective-target tls } */
> > > +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> > > +
> > > +
> > > +#pragma GCC visibility push(hidden)
> > > +
> > > +// tls_model should be local-dynamic due to a pragma
> > > +__thread int x;
> > > +
> > > +#pragma GCC visibility pop
> > > +
> > > +void reference() { x++; }
> > > +
> > > +/* { dg-final { scan-ipa-dump "Varpool flags: tls-local-dynamic" "whole-program" } } */
> > > 
> > 
>
  

Hi,

sorry for ignoring this for so long, I hope that Honza wold take over.

I think the patch would be good if it did not have....

On Thu, Jul 07 2022, Alexander Monakov via Gcc-patches wrote:
> From: Artem Klimov <jakmobius@gmail.com>
>
> Fix PR99619, which asks to optimize TLS model based on visibility.
> The fix is implemented as an IPA optimization: this allows to take
> optimized visibility status into account (as well as avoid modifying
> all language frontends).
>
> 2022-04-17  Artem Klimov  <jakmobius@gmail.com>
>
> gcc/ChangeLog:
>
> 	* ipa-visibility.cc (function_and_variable_visibility): Promote
> 	TLS access model afer visibility optimizations.
> 	* varasm.cc (have_optimized_refs): New helper.
> 	(optimize_dyn_tls_for_decl_p): New helper. Use it ...
> 	(decl_default_tls_model): ... here in place of 'optimize' check.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.dg/tls/vis-attr-gd.c: New test.
> 	* gcc.dg/tls/vis-attr-hidden-gd.c: New test.
> 	* gcc.dg/tls/vis-attr-hidden.c: New test.
> 	* gcc.dg/tls/vis-flag-hidden-gd.c: New test.
> 	* gcc.dg/tls/vis-flag-hidden.c: New test.
> 	* gcc.dg/tls/vis-pragma-hidden-gd.c: New test.
> 	* gcc.dg/tls/vis-pragma-hidden.c: New test.
>
> Co-Authored-By:  Alexander Monakov  <amonakov@gcc.gnu.org>
> Signed-off-by: Artem Klimov <jakmobius@gmail.com>
> ---
>
> v2: run the new loop in ipa-visibility only in the whole-program IPA pass;
>     in decl_default_tls_model, check if any referring function is optimized
>     when 'optimize == 0' (when running in LTO mode)
>
>
> Note for reviewers: I noticed there's a place which tries to avoid TLS
> promotion, but the comment seems wrong and I could not find a testcase.
> I'd suggest we remove it. The compiler can only promote general-dynamic
> to local-dynamic and initial-exec to local-exec. The comment refers to
> promoting x-dynamic to y-exec, but that cannot happen AFAICT:
> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=8e1ba78f1b8eedd6c65c6f0e6d6d09a801de5d3d
>
>
>  gcc/ipa-visibility.cc                         | 19 +++++++++++
>  gcc/testsuite/gcc.dg/tls/vis-attr-gd.c        | 12 +++++++
>  gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c | 13 ++++++++
>  gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c    | 12 +++++++
>  gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c | 13 ++++++++
>  gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c    | 12 +++++++
>  .../gcc.dg/tls/vis-pragma-hidden-gd.c         | 17 ++++++++++
>  gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c  | 16 ++++++++++
>  gcc/varasm.cc                                 | 32 ++++++++++++++++++-
>  9 files changed, 145 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
>
> diff --git a/gcc/ipa-visibility.cc b/gcc/ipa-visibility.cc
> index 8a27e7bcd..3ed2b7cf6 100644
> --- a/gcc/ipa-visibility.cc
> +++ b/gcc/ipa-visibility.cc
> @@ -873,6 +873,25 @@ function_and_variable_visibility (bool whole_program)
>  	}
>      }
>  
> +  if (symtab->state >= IPA_SSA)
> +    {
> +      FOR_EACH_VARIABLE (vnode)
> +	{
> +	  tree decl = vnode->decl;
> +
> +	  /* Upgrade TLS access model based on optimized visibility status,
> +	     unless it was specified explicitly or no references remain.  */
> +	  if (DECL_THREAD_LOCAL_P (decl)
> +	      && !lookup_attribute ("tls_model", DECL_ATTRIBUTES (decl))
> +	      && vnode->ref_list.referring.length ())
> +	    {
> +	      enum tls_model new_model = decl_default_tls_model (decl);
> +	      gcc_checking_assert (new_model >= decl_tls_model (decl));
> +	      set_decl_tls_model (decl, new_model);
> +	    }
> +	}
> +    }
> +
>    if (dump_file)
>      {
>        fprintf (dump_file, "\nMarking local functions:");
> diff --git a/gcc/varasm.cc b/gcc/varasm.cc
> index 4db8506b1..de149e82c 100644
> --- a/gcc/varasm.cc
> +++ b/gcc/varasm.cc
> @@ -6679,6 +6679,36 @@ init_varasm_once (void)
>  #endif
>  }
>  
> +/* Determine whether SYMBOL is used in any optimized function.  */
> +
> +static bool
> +have_optimized_refs (struct symtab_node *symbol)
> +{
> +  struct ipa_ref *ref;
> +
> +  for (int i = 0; symbol->iterate_referring (i, ref); i++)
> +    {
> +      cgraph_node *cnode = dyn_cast <cgraph_node *> (ref->referring);
> +
> +      if (cnode && opt_for_fn (cnode->decl, optimize))
> +	return true;
> +    }
> +
> +  return false;
> +}
> +
> +/* Check if promoting general-dynamic TLS access model to local-dynamic is
> +   desirable for DECL.  */
> +
> +static bool
> +optimize_dyn_tls_for_decl_p (const_tree decl)
> +{
> +  if (optimize)
> +    return true;

...this.  This is again an access to optimize which in LTO IPA phase is
not really meaningful.  Can the test be simply removed?  If not (but
please look at why), I'd suggest to test the overall optimize level only
if there is a non-NULL cfun.

Otherwise, the patch looks OK to me.

Martin

> +  return symtab->state >= IPA && have_optimized_refs (symtab_node::get (decl));
> +}
> +
> +
>  enum tls_model
>  decl_default_tls_model (const_tree decl)
>  {
> @@ -6696,7 +6726,7 @@ decl_default_tls_model (const_tree decl)
>  
>    /* Local dynamic is inefficient when we're not combining the
>       parts of the address.  */
> -  else if (optimize && is_local)
> +  else if (is_local && optimize_dyn_tls_for_decl_p (decl))
>      kind = TLS_MODEL_LOCAL_DYNAMIC;
>    else
>      kind = TLS_MODEL_GLOBAL_DYNAMIC;
  

On Fri, 26 Aug 2022, Martin Jambor wrote:

> > +/* Check if promoting general-dynamic TLS access model to local-dynamic is
> > +   desirable for DECL.  */
> > +
> > +static bool
> > +optimize_dyn_tls_for_decl_p (const_tree decl)
> > +{
> > +  if (optimize)
> > +    return true;
> 
> ...this.  This is again an access to optimize which in LTO IPA phase is
> not really meaningful.  Can the test be simply removed?  If not (but
> please look at why), I'd suggest to test the overall optimize level only
> if there is a non-NULL cfun.

I'd prefer to keep it. This code is also called from the front-ends when
assigning initial TLS access model (during parsing, at the point where
visibility attributes, if present, have not been processed yet). There
we don't have IPA structures, but 'optimize' is set up.

I also want to avoid iterating over referring functions in non-LTO mode
where trusting 'optimize' should be fine during this IPA pass I think?

Alexander
  

On Fri, Aug 26 2022, Alexander Monakov wrote:
> On Fri, 26 Aug 2022, Martin Jambor wrote:
>
>> > +/* Check if promoting general-dynamic TLS access model to local-dynamic is
>> > +   desirable for DECL.  */
>> > +
>> > +static bool
>> > +optimize_dyn_tls_for_decl_p (const_tree decl)
>> > +{
>> > +  if (optimize)
>> > +    return true;
>> 
>> ...this.  This is again an access to optimize which in LTO IPA phase is
>> not really meaningful.  Can the test be simply removed?  If not (but
>> please look at why), I'd suggest to test the overall optimize level only
>> if there is a non-NULL cfun.
>
> I'd prefer to keep it. This code is also called from the front-ends when
> assigning initial TLS access model (during parsing, at the point where
> visibility attributes, if present, have not been processed yet). There
> we don't have IPA structures, but 'optimize' is set up.
>
> I also want to avoid iterating over referring functions in non-LTO mode
> where trusting 'optimize' should be fine during this IPA pass I think?

There is still the optimize attribute so in fact no, even in non-LTO
mode if there is no current function, you cannot trust the "global"
"optimize" thing.

Ideally we would assert that no "analysis" phase of an IPA pass reads
the global optimization flags, so please don't add new places where we
do.

You can either add a parameter to decl_default_tls_model to tell it
what context it is called from and IMHO it would also be acceptable to
check whether we have a non-NULL cfun and decide based on that (but here
I only hope it is not something others might object to).

Martin
  

On Tue, 30 Aug 2022, Martin Jambor wrote:

> There is still the optimize attribute so in fact no, even in non-LTO
> mode if there is no current function, you cannot trust the "global"
> "optimize" thing.
> 
> Ideally we would assert that no "analysis" phase of an IPA pass reads
> the global optimization flags, so please don't add new places where we
> do.
> 
> You can either add a parameter to decl_default_tls_model to tell it
> what context it is called from and IMHO it would also be acceptable to
> check whether we have a non-NULL cfun and decide based on that (but here
> I only hope it is not something others might object to).

I see, thank you for explaining the issue, and sorry if I was a bit stubborn.

Does the attached patch (incremental change below) look better? It no longer
has the 'shortcut' where iterating over referrers is avoided for the common
case of plain 'gcc -O2' and no 'optimize' attributes, but fortunately TLS
variables are not so numerous to make chasing that worthwhile.

--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -6703,8 +6703,8 @@ have_optimized_refs (struct symtab_node *symbol)
 static bool
 optimize_dyn_tls_for_decl_p (const_tree decl)
 {
-  if (optimize)
-    return true;
+  if (cfun)
+    return optimize;
   return symtab->state >= IPA && have_optimized_refs (symtab_node::get (decl));
 }

Alexander
  

> I see, thank you for explaining the issue, and sorry if I was a bit stubborn.
> 
> Does the attached patch (incremental change below) look better? It no longer
> has the 'shortcut' where iterating over referrers is avoided for the common
> case of plain 'gcc -O2' and no 'optimize' attributes, but fortunately TLS
> variables are not so numerous to make chasing that worthwhile.

... and of course I forgot to add the attachment.  Revised patch below.

---8<---

From b245015ec465604799aef60b224b1e1e264d4cb8 Mon Sep 17 00:00:00 2001
From: Artem Klimov <jakmobius@gmail.com>
Date: Wed, 6 Jul 2022 17:02:01 +0300
Subject: [PATCH] ipa-visibility: Optimize TLS access [PR99619]

Fix PR99619, which asks to optimize TLS model based on visibility.
The fix is implemented as an IPA optimization: this allows to take
optimized visibility status into account (as well as avoid modifying
all language frontends).

2022-04-17  Artem Klimov  <jakmobius@gmail.com>

gcc/ChangeLog:

	* ipa-visibility.cc (function_and_variable_visibility): Promote
	TLS access model afer visibility optimizations.
	* varasm.cc (have_optimized_refs): New helper.
	(optimize_dyn_tls_for_decl_p): New helper. Use it ...
	(decl_default_tls_model): ... here in place of 'optimize' check.

gcc/testsuite/ChangeLog:

	* gcc.dg/tls/vis-attr-gd.c: New test.
	* gcc.dg/tls/vis-attr-hidden-gd.c: New test.
	* gcc.dg/tls/vis-attr-hidden.c: New test.
	* gcc.dg/tls/vis-flag-hidden-gd.c: New test.
	* gcc.dg/tls/vis-flag-hidden.c: New test.
	* gcc.dg/tls/vis-pragma-hidden-gd.c: New test.
	* gcc.dg/tls/vis-pragma-hidden.c: New test.

Co-Authored-By:  Alexander Monakov  <amonakov@gcc.gnu.org>
Signed-off-by: Artem Klimov <jakmobius@gmail.com>
---
 gcc/ipa-visibility.cc                         | 19 +++++++++++
 gcc/testsuite/gcc.dg/tls/vis-attr-gd.c        | 12 +++++++
 gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c | 13 ++++++++
 gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c    | 12 +++++++
 gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c | 13 ++++++++
 gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c    | 12 +++++++
 .../gcc.dg/tls/vis-pragma-hidden-gd.c         | 17 ++++++++++
 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c  | 16 ++++++++++
 gcc/varasm.cc                                 | 32 ++++++++++++++++++-
 9 files changed, 145 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
 create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
 create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
 create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
 create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
 create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
 create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c

diff --git a/gcc/ipa-visibility.cc b/gcc/ipa-visibility.cc
index 8a27e7bcd..3ed2b7cf6 100644
--- a/gcc/ipa-visibility.cc
+++ b/gcc/ipa-visibility.cc
@@ -873,6 +873,25 @@ function_and_variable_visibility (bool whole_program)
 	}
     }
 
+  if (symtab->state >= IPA_SSA)
+    {
+      FOR_EACH_VARIABLE (vnode)
+	{
+	  tree decl = vnode->decl;
+
+	  /* Upgrade TLS access model based on optimized visibility status,
+	     unless it was specified explicitly or no references remain.  */
+	  if (DECL_THREAD_LOCAL_P (decl)
+	      && !lookup_attribute ("tls_model", DECL_ATTRIBUTES (decl))
+	      && vnode->ref_list.referring.length ())
+	    {
+	      enum tls_model new_model = decl_default_tls_model (decl);
+	      gcc_checking_assert (new_model >= decl_tls_model (decl));
+	      set_decl_tls_model (decl, new_model);
+	    }
+	}
+    }
+
   if (dump_file)
     {
       fprintf (dump_file, "\nMarking local functions:");
diff --git a/gcc/varasm.cc b/gcc/varasm.cc
index 4db8506b1..ffc559431 100644
--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -6679,6 +6679,36 @@ init_varasm_once (void)
 #endif
 }
 
+/* Determine whether SYMBOL is used in any optimized function.  */
+
+static bool
+have_optimized_refs (struct symtab_node *symbol)
+{
+  struct ipa_ref *ref;
+
+  for (int i = 0; symbol->iterate_referring (i, ref); i++)
+    {
+      cgraph_node *cnode = dyn_cast <cgraph_node *> (ref->referring);
+
+      if (cnode && opt_for_fn (cnode->decl, optimize))
+	return true;
+    }
+
+  return false;
+}
+
+/* Check if promoting general-dynamic TLS access model to local-dynamic is
+   desirable for DECL.  */
+
+static bool
+optimize_dyn_tls_for_decl_p (const_tree decl)
+{
+  if (cfun)
+    return optimize;
+  return symtab->state >= IPA && have_optimized_refs (symtab_node::get (decl));
+}
+
+
 enum tls_model
 decl_default_tls_model (const_tree decl)
 {
@@ -6696,7 +6726,7 @@ decl_default_tls_model (const_tree decl)
 
   /* Local dynamic is inefficient when we're not combining the
      parts of the address.  */
-  else if (optimize && is_local)
+  else if (is_local && optimize_dyn_tls_for_decl_p (decl))
     kind = TLS_MODEL_LOCAL_DYNAMIC;
   else
     kind = TLS_MODEL_GLOBAL_DYNAMIC;
diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c b/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
new file mode 100644
index 000000000..89a248a80
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target fpic } */
+/* { dg-require-effective-target tls } */
+/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
+
+// tls_model should be global-dynamic due to explicitly specified attribute
+__attribute__((tls_model("global-dynamic")))
+__thread int x;
+
+void reference() { x++; }
+
+/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
new file mode 100644
index 000000000..e32565588
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target fpic } */
+/* { dg-require-effective-target tls } */
+/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
+
+// tls_model should be global-dynamic due to explicitly specified attribute
+__attribute__((visibility("hidden")))
+__attribute__((tls_model("global-dynamic")))
+__thread int x;
+
+void reference() { x++; }
+
+/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
new file mode 100644
index 000000000..0d43fc565
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target fpic } */
+/* { dg-require-effective-target tls } */
+/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
+
+//tls_model should be local-dynamic due to visibility("hidden")
+__attribute__((visibility("hidden")))
+__thread int x;
+
+void reference() { x++; }
+
+/* { dg-final { scan-ipa-dump "Varpool flags: tls-local-dynamic" "whole-program" } } */
diff --git a/gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
new file mode 100644
index 000000000..cad41e0c8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target fpic } */
+/* { dg-require-effective-target tls } */
+/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program -fvisibility=hidden" } */
+
+
+// tls_model should be global-dynamic due to explicitly specified attribute
+__attribute__((tls_model("global-dynamic")))
+__thread int x;
+
+void reference() { x++; }
+
+/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
diff --git a/gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
new file mode 100644
index 000000000..a15df092d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target fpic } */
+/* { dg-require-effective-target tls } */
+/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program -fvisibility=hidden" } */
+
+
+// tls_model should be local-dynamic due to -fvisibility=hidden
+__thread int x;
+
+void reference() { x++; }
+
+/* { dg-final { scan-ipa-dump "Varpool flags: tls-local-dynamic" "whole-program" } } */
diff --git a/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
new file mode 100644
index 000000000..3b3598134
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target fpic } */
+/* { dg-require-effective-target tls } */
+/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
+
+
+#pragma GCC visibility push(hidden)
+
+// tls_model should be global-dynamic due to explicitly specified attribute
+__attribute__((tls_model("global-dynamic")))
+__thread int x;
+
+#pragma GCC visibility pop
+
+void reference() { x++; }
+
+/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
diff --git a/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
new file mode 100644
index 000000000..1be976442
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target fpic } */
+/* { dg-require-effective-target tls } */
+/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
+
+
+#pragma GCC visibility push(hidden)
+
+// tls_model should be local-dynamic due to a pragma
+__thread int x;
+
+#pragma GCC visibility pop
+
+void reference() { x++; }
+
+/* { dg-final { scan-ipa-dump "Varpool flags: tls-local-dynamic" "whole-program" } } */
  

Hi,

On Tue, Aug 30 2022, Alexander Monakov wrote:
>> I see, thank you for explaining the issue, and sorry if I was a bit stubborn.
>> 
>> Does the attached patch (incremental change below) look better? It no longer
>> has the 'shortcut' where iterating over referrers is avoided for the common
>> case of plain 'gcc -O2' and no 'optimize' attributes, but fortunately TLS
>> variables are not so numerous to make chasing that worthwhile.
>
> ... and of course I forgot to add the attachment.  Revised patch below.

I hope I am not overstepping my IPA/cgraph maintainer authority here but
I believe the patch is OK for master (assuming it passes bootstrap and
testing).

Thanks,

Martin


>
> ---8<---
>
> From b245015ec465604799aef60b224b1e1e264d4cb8 Mon Sep 17 00:00:00 2001
> From: Artem Klimov <jakmobius@gmail.com>
> Date: Wed, 6 Jul 2022 17:02:01 +0300
> Subject: [PATCH] ipa-visibility: Optimize TLS access [PR99619]
>
> Fix PR99619, which asks to optimize TLS model based on visibility.
> The fix is implemented as an IPA optimization: this allows to take
> optimized visibility status into account (as well as avoid modifying
> all language frontends).
>
> 2022-04-17  Artem Klimov  <jakmobius@gmail.com>
>
> gcc/ChangeLog:
>
> 	* ipa-visibility.cc (function_and_variable_visibility): Promote
> 	TLS access model afer visibility optimizations.
> 	* varasm.cc (have_optimized_refs): New helper.
> 	(optimize_dyn_tls_for_decl_p): New helper. Use it ...
> 	(decl_default_tls_model): ... here in place of 'optimize' check.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.dg/tls/vis-attr-gd.c: New test.
> 	* gcc.dg/tls/vis-attr-hidden-gd.c: New test.
> 	* gcc.dg/tls/vis-attr-hidden.c: New test.
> 	* gcc.dg/tls/vis-flag-hidden-gd.c: New test.
> 	* gcc.dg/tls/vis-flag-hidden.c: New test.
> 	* gcc.dg/tls/vis-pragma-hidden-gd.c: New test.
> 	* gcc.dg/tls/vis-pragma-hidden.c: New test.
>
> Co-Authored-By:  Alexander Monakov  <amonakov@gcc.gnu.org>
> Signed-off-by: Artem Klimov <jakmobius@gmail.com>
> ---
>  gcc/ipa-visibility.cc                         | 19 +++++++++++
>  gcc/testsuite/gcc.dg/tls/vis-attr-gd.c        | 12 +++++++
>  gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c | 13 ++++++++
>  gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c    | 12 +++++++
>  gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c | 13 ++++++++
>  gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c    | 12 +++++++
>  .../gcc.dg/tls/vis-pragma-hidden-gd.c         | 17 ++++++++++
>  gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c  | 16 ++++++++++
>  gcc/varasm.cc                                 | 32 ++++++++++++++++++-
>  9 files changed, 145 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
>  create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
>
> diff --git a/gcc/ipa-visibility.cc b/gcc/ipa-visibility.cc
> index 8a27e7bcd..3ed2b7cf6 100644
> --- a/gcc/ipa-visibility.cc
> +++ b/gcc/ipa-visibility.cc
> @@ -873,6 +873,25 @@ function_and_variable_visibility (bool whole_program)
>  	}
>      }
>  
> +  if (symtab->state >= IPA_SSA)
> +    {
> +      FOR_EACH_VARIABLE (vnode)
> +	{
> +	  tree decl = vnode->decl;
> +
> +	  /* Upgrade TLS access model based on optimized visibility status,
> +	     unless it was specified explicitly or no references remain.  */
> +	  if (DECL_THREAD_LOCAL_P (decl)
> +	      && !lookup_attribute ("tls_model", DECL_ATTRIBUTES (decl))
> +	      && vnode->ref_list.referring.length ())
> +	    {
> +	      enum tls_model new_model = decl_default_tls_model (decl);
> +	      gcc_checking_assert (new_model >= decl_tls_model (decl));
> +	      set_decl_tls_model (decl, new_model);
> +	    }
> +	}
> +    }
> +
>    if (dump_file)
>      {
>        fprintf (dump_file, "\nMarking local functions:");
> diff --git a/gcc/varasm.cc b/gcc/varasm.cc
> index 4db8506b1..ffc559431 100644
> --- a/gcc/varasm.cc
> +++ b/gcc/varasm.cc
> @@ -6679,6 +6679,36 @@ init_varasm_once (void)
>  #endif
>  }
>  
> +/* Determine whether SYMBOL is used in any optimized function.  */
> +
> +static bool
> +have_optimized_refs (struct symtab_node *symbol)
> +{
> +  struct ipa_ref *ref;
> +
> +  for (int i = 0; symbol->iterate_referring (i, ref); i++)
> +    {
> +      cgraph_node *cnode = dyn_cast <cgraph_node *> (ref->referring);
> +
> +      if (cnode && opt_for_fn (cnode->decl, optimize))
> +	return true;
> +    }
> +
> +  return false;
> +}
> +
> +/* Check if promoting general-dynamic TLS access model to local-dynamic is
> +   desirable for DECL.  */
> +
> +static bool
> +optimize_dyn_tls_for_decl_p (const_tree decl)
> +{
> +  if (cfun)
> +    return optimize;
> +  return symtab->state >= IPA && have_optimized_refs (symtab_node::get (decl));
> +}
> +
> +
>  enum tls_model
>  decl_default_tls_model (const_tree decl)
>  {
> @@ -6696,7 +6726,7 @@ decl_default_tls_model (const_tree decl)
>  
>    /* Local dynamic is inefficient when we're not combining the
>       parts of the address.  */
> -  else if (optimize && is_local)
> +  else if (is_local && optimize_dyn_tls_for_decl_p (decl))
>      kind = TLS_MODEL_LOCAL_DYNAMIC;
>    else
>      kind = TLS_MODEL_GLOBAL_DYNAMIC;
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c b/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
> new file mode 100644
> index 000000000..89a248a80
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> +
> +// tls_model should be global-dynamic due to explicitly specified attribute
> +__attribute__((tls_model("global-dynamic")))
> +__thread int x;
> +
> +void reference() { x++; }
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
> new file mode 100644
> index 000000000..e32565588
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> +
> +// tls_model should be global-dynamic due to explicitly specified attribute
> +__attribute__((visibility("hidden")))
> +__attribute__((tls_model("global-dynamic")))
> +__thread int x;
> +
> +void reference() { x++; }
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
> new file mode 100644
> index 000000000..0d43fc565
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> +
> +//tls_model should be local-dynamic due to visibility("hidden")
> +__attribute__((visibility("hidden")))
> +__thread int x;
> +
> +void reference() { x++; }
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-local-dynamic" "whole-program" } } */
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
> new file mode 100644
> index 000000000..cad41e0c8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program -fvisibility=hidden" } */
> +
> +
> +// tls_model should be global-dynamic due to explicitly specified attribute
> +__attribute__((tls_model("global-dynamic")))
> +__thread int x;
> +
> +void reference() { x++; }
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
> new file mode 100644
> index 000000000..a15df092d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program -fvisibility=hidden" } */
> +
> +
> +// tls_model should be local-dynamic due to -fvisibility=hidden
> +__thread int x;
> +
> +void reference() { x++; }
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-local-dynamic" "whole-program" } } */
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
> new file mode 100644
> index 000000000..3b3598134
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> +
> +
> +#pragma GCC visibility push(hidden)
> +
> +// tls_model should be global-dynamic due to explicitly specified attribute
> +__attribute__((tls_model("global-dynamic")))
> +__thread int x;
> +
> +#pragma GCC visibility pop
> +
> +void reference() { x++; }
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic" "whole-program" } } */
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
> new file mode 100644
> index 000000000..1be976442
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-whole-program" } */
> +
> +
> +#pragma GCC visibility push(hidden)
> +
> +// tls_model should be local-dynamic due to a pragma
> +__thread int x;
> +
> +#pragma GCC visibility pop
> +
> +void reference() { x++; }
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-local-dynamic" "whole-program" } } */
> -- 
> 2.35.1