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 <jakmob...@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 <jakmob...@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 <amona...@gcc.gnu.org> > > > Signed-off-by: Artem Klimov <jakmob...@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" } } */ > > > > > >
