On Mon, Sep 8, 2025 at 11:50 PM Alexander Monakov <amona...@ispras.ru> wrote:
>
>
> On Mon, 8 Sep 2025, H.J. Lu wrote:
>
> > Don't check tls_model attribute when optimizing TLS access since
> >
> > 1. -ftls-model=initial-exec also can specify tls_model without tls_model
> > attribute.
>
> There's a difference: -ftls-model is usable only for upgrading the model;
> if you pass -ftls-model=global-dynamic without -fpic, you still get local-exec
> or initial-exec. Just a correction in case you missed it.
True, just like tls_model attribute, -ftls-model specifies the weakest TLS
access model.
> > 2. Linker can optimize TLS access at link-time.
> > 3. LTO should perform the similar optimization.
>
> I generally agree that GCC should perform this optimization. That said, with
> this patch we arrive at a situation with both
>
> a) ld.bfd does not obey --no-relax to switch off TLS relaxation, and
> b) gcc unconditionally performs TLS access optimization
>
> which makes investigation of TLS-related toolchain bugs difficult. Would you
> consider adding a GCC flag to control this optimization? And using the flag
> in the testcases?
Will do.
> > Since C, C++, and Fortran front-ends now set the TLS access model after
> > a variable has been fully processed, not in the middle of processing it,
> > partially revert
> >
> > commit 82e629c26647313be406c41a01e6868cfad0f289
> > Author: Alexander Monakov <amona...@ispras.ru>
> > Date: Wed Oct 26 16:37:34 2022 +0300
> >
> > ipa-visibility: remove assert in TLS optimization [PR107353]
> >
> > to restore assert if not compiling for shared library. When compiling
> > for shared library, the recomputed model may be weaker.
>
> I don't understand why (for shared libraries), can you explain?
Another shared library or executable may override any global
symbols with the default visibility in a shared library.
> > Linker may
> > issue an error if the front-end assigned model isn't supported in shared
> > library. But compiler doesn't know if the generated code will be linked
> > into executable or shared library.
> >
> > gcc/
> >
> > PR other/107353
> > PR middle-end/121352
> > * ipa-visibility.cc (function_and_variable_visibility): Don't
> > check tls_model attribute. Restore assert if not compiling for
> > shared library.
> > * doc/extend.texi: Update the tls_model attribute.
> >
> > gcc/testsuite/
> >
> > PR middle-end/121352
> > * gcc.dg/tls/vis-attr-hidden-gd.c: Scan for tls-local-dynamic.
> > * gcc.dg/tls/vis-flag-hidden-gd.c: Likewise.
> > * gcc.dg/tls/vis-pragma-hidden-gd.c: Likewise.
> >
> > Signed-off-by: H.J. Lu <hjl.to...@gmail.com>
> > ---
> > gcc/doc/extend.texi | 5 ++++-
> > gcc/ipa-visibility.cc | 19 ++++++++++++-------
> > gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c | 4 ++--
> > gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c | 4 ++--
> > .../gcc.dg/tls/vis-pragma-hidden-gd.c | 4 ++--
> > 5 files changed, 22 insertions(+), 14 deletions(-)
> >
> > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > index 2922d9e9839..879a759714b 100644
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -7563,7 +7563,10 @@ The @code{tls_model} attribute sets thread-local
> > storage model
> > overriding @option{-ftls-model=} command-line switch on a per-variable
> > basis.
> > The @var{tls_model} argument should be one of @code{global-dynamic},
> > -@code{local-dynamic}, @code{initial-exec} or @code{local-exec}.
> > +@code{local-dynamic}, @code{initial-exec} or @code{local-exec}. The
> > +@code{tls_model} attribute specifies the weakest @acronym{TLS} access
> > +model. GCC may optimize @acronym{TLS} access to a stronger @acronym{TLS}
> > +access model.
>
> I think with this patch the attribute does not fully override the -ftls-model
> option. If so, the 'overriding' claim in the previous paragraph should be
> changed. If you add a command-line flag to control the optimization, this
> section will have to be reworded.
Will do.
> >
> > Not all targets support this attribute.
> >
> > diff --git a/gcc/ipa-visibility.cc b/gcc/ipa-visibility.cc
> > index 8097a03e240..d2283507a31 100644
> > --- a/gcc/ipa-visibility.cc
> > +++ b/gcc/ipa-visibility.cc
> > @@ -883,17 +883,22 @@ function_and_variable_visibility (bool whole_program)
> > tree decl = vnode->decl;
> >
> > /* Upgrade TLS access model based on optimized visibility status,
> > - unless it was specified explicitly or no references remain. */
> > + unless 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);
> > - STATIC_ASSERT (TLS_MODEL_GLOBAL_DYNAMIC <
> > TLS_MODEL_LOCAL_DYNAMIC);
> > - STATIC_ASSERT (TLS_MODEL_INITIAL_EXEC < TLS_MODEL_LOCAL_EXEC);
> > - /* We'd prefer to assert that recomputed model is not weaker
> > than
> > - what the front-end assigned, but cannot: see PR 107353. */
> > - if (new_model >= decl_tls_model (decl))
> > + /* Assert that the recomputed model is not weaker than
> > + what the front-end assigned if not comping for shared
> > + library. When compiling for shared library, the
> > + recomputed model may be weaker. Linker may issue an
> > + error if the front-end assigned model isn't supported
> > + in shared library. But compiler doesn't know if the
> > + generated code will be linked into executable or shared
> > + library. */
> > + tls_model model = decl_tls_model (decl);
> > + gcc_checking_assert (flag_shlib || new_model >= model);
>
> I think this can trip if a variable is redeclared such that initial
> declaration
> implies a stronger model:
>
> // compile with gcc -O2 -std=c11 -fno-common
> __thread int x;
>
> __attribute__((common))
> __thread int x;
>
> int *f()
> {
> return &x;
> }
>
> Can you check?
>
[hjl@gnu-zen4-1 gcc]$ cat /tmp/x.c
__thread int x;
__attribute__((common))
__thread int x;
int *f()
{
return &x;
}
[hjl@gnu-zen4-1 gcc]$ ./xgcc -B./ -O2 -std=c11 -fno-common -S /tmp/x.c
-fdump-ipa-whole-program
[hjl@gnu-zen4-1 gcc]$ cat x.c.084i.whole-program
Marking local functions:
Marking externally visible functions: f/3
Marking externally visible variables: x/1
Clearing variable flags:
Reclaiming functions:
Reclaiming variables:
Clearing address taken flags:
Symbol table:
f/3 (f)
Type: function definition analyzed
Visibility: externally_visible semantic_interposition public
References: x/1 (addr)
Referring:
Availability: available
Function flags: count:1073741824 (estimated locally) body
Called by:
Calls:
x/1 (x)
Type: variable definition analyzed
Visibility: externally_visible semantic_interposition public common
References:
Referring: f/3 (addr)
Availability: overwritable
Varpool flags: tls-local-exec
__attribute__((returns_nonnull))
int * f ()
{
<bb 2> [local count: 1073741824]:
return &x;
}
[hjl@gnu-zen4-1 gcc]$
--
H.J.
