public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] ipa-visibility: remove assert in TLS optimization [PR107353]
@ 2022-10-26 13:52 Alexander Monakov
  2022-10-26 15:58 ` Martin Jambor
  0 siblings, 1 reply; 2+ messages in thread
From: Alexander Monakov @ 2022-10-26 13:52 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jan Hubicka, Artem Klimov, Alexander Monakov

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(-)

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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] ipa-visibility: remove assert in TLS optimization [PR107353]
  2022-10-26 13:52 [PATCH] ipa-visibility: remove assert in TLS optimization [PR107353] Alexander Monakov
@ 2022-10-26 15:58 ` Martin Jambor
  0 siblings, 0 replies; 2+ messages in thread
From: Martin Jambor @ 2022-10-26 15:58 UTC (permalink / raw)
  To: Alexander Monakov, gcc-patches
  Cc: Jan Hubicka, Alexander Monakov, Artem Klimov

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-10-26 15:58 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-26 13:52 [PATCH] ipa-visibility: remove assert in TLS optimization [PR107353] Alexander Monakov
2022-10-26 15:58 ` Martin Jambor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).