ipa-visibility: remove assert in TLS optimization [PR107353]
Checks
Commit Message
When upgrading TLS access model based on optimized symbol visibility
status, we attempted to assert that recomputing the model would not
weaken it. It turns out that C, C++, and Fortran front-ends all can
(unintentionally) assign a stronger model than what can be derived
from the declaration.
Let's act conservatively instead of asserting, at least as long as
such pre-existing issues remain.
gcc/ChangeLog:
PR other/107353
* ipa-visibility.cc (function_and_variable_visibility):
Conditionally upgrade TLS model instead of asserting.
---
gcc/ipa-visibility.cc | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
Comments
Hi,
On Wed, Oct 26 2022, Alexander Monakov wrote:
> When upgrading TLS access model based on optimized symbol visibility
> status, we attempted to assert that recomputing the model would not
> weaken it. It turns out that C, C++, and Fortran front-ends all can
> (unintentionally) assign a stronger model than what can be derived
> from the declaration.
If you believe that FEs assign a wrong model sometimes (that is my
impression after reading your bugzilla comments), please open bugs for
those cases.
>
> Let's act conservatively instead of asserting, at least as long as
> such pre-existing issues remain.
>
> gcc/ChangeLog:
>
> PR other/107353
> * ipa-visibility.cc (function_and_variable_visibility):
> Conditionally upgrade TLS model instead of asserting.
The patch is OK (assuming it passes bootstrap&testing).
Thanks,
Martin
> ---
> gcc/ipa-visibility.cc | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/ipa-visibility.cc b/gcc/ipa-visibility.cc
> index 3ed2b7cf6..238f7eb84 100644
> --- a/gcc/ipa-visibility.cc
> +++ b/gcc/ipa-visibility.cc
> @@ -886,8 +886,12 @@ function_and_variable_visibility (bool whole_program)
> && 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);
> + 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))
> + set_decl_tls_model (decl, new_model);
> }
> }
> }
> --
> 2.37.2
@@ -886,8 +886,12 @@ function_and_variable_visibility (bool whole_program)
&& 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);
+ 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))
+ set_decl_tls_model (decl, new_model);
}
}
}