public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r14-8962] c++/modules: reduce lazy loading recursion
@ 2024-02-13 19:27 Patrick Palka
  0 siblings, 0 replies; only message in thread
From: Patrick Palka @ 2024-02-13 19:27 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:ce67b75e918bcbd31ed32bc820f5233c72670feb

commit r14-8962-gce67b75e918bcbd31ed32bc820f5233c72670feb
Author: Patrick Palka <ppalka@redhat.com>
Date:   Tue Feb 13 14:26:37 2024 -0500

    c++/modules: reduce lazy loading recursion
    
    It turns out that with modules we can call mangle_decl recursively
    which is bad because the global mangling state isn't recursion aware.
    The recursion happens from write_closure_type_name, which calls
    lambda_function, which performs name lookup, which can trigger lazy
    loading, which can call maybe_clone_body for a newly loaded cdtor, which
    can inspect DECL_ASSEMBLER_NAME, which enters mangling.  This was
    observed when using fmtlib as a module with trunk and it leads to a
    bogus "mangling conflicts with a previous mangle error" followed by an
    ICE from cdtor_comdat_group due to a mangling mismatch.
    
    This patch fixes this by sidestepping lazy loading when performing the
    op() lookup in lambda_function, so that we don't accidentally end up
    entering mangling recursively.  This should be safe since the lazy load
    should still get triggered by some other name lookup.
    
    In passing it was noticed that lazy loading can get excessively recursive
    ultimately due to the name lookups performed from check_local_shadow,
    which may trigger lazy loading, which cause us to instantiate/clone
    things, which end up calling check_local_shadow.  This patch mitigates
    this by implementating an optimization suggested by Jason:
    
    > I think we shouldn't bother with check_local_shadow in a clone or
    > instantiation, only when actually parsing.
    
    This reduces the maximum depth of lazy loading recursion for a simple
    modular Hello World from ~115 to ~12.
    
    gcc/cp/ChangeLog:
    
            * lambda.cc (lambda_function): Call get_class_binding_direct
            instead of lookup_member to sidestep lazy loading.
            * name-lookup.cc (check_local_shadow): Punt if we're in a
            function context that's not actual parsing.
    
    Reviewed-by: Jason Merill <jason@redhat.com>

Diff:
---
 gcc/cp/lambda.cc      |  4 +---
 gcc/cp/name-lookup.cc | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc
index 1d37e5a52b98..4b1f9391fee1 100644
--- a/gcc/cp/lambda.cc
+++ b/gcc/cp/lambda.cc
@@ -175,9 +175,7 @@ lambda_function (tree lambda)
   if (CLASSTYPE_TEMPLATE_INSTANTIATION (type)
       && !COMPLETE_OR_OPEN_TYPE_P (type))
     return NULL_TREE;
-  lambda = lookup_member (type, call_op_identifier,
-			  /*protect=*/0, /*want_type=*/false,
-			  tf_warning_or_error);
+  lambda = get_class_binding_direct (type, call_op_identifier);
   if (lambda)
     lambda = STRIP_TEMPLATE (get_first_fn (lambda));
   return lambda;
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index e58f3b5cb4d4..6444db3f0ebf 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -3275,6 +3275,23 @@ check_local_shadow (tree decl)
   if (TREE_CODE (decl) == PARM_DECL && !DECL_CONTEXT (decl))
     return NULL_TREE;
 
+  if (DECL_FUNCTION_SCOPE_P (decl))
+    {
+      tree ctx = DECL_CONTEXT (decl);
+      if (DECL_CLONED_FUNCTION_P (ctx)
+	  || DECL_TEMPLATE_INSTANTIATED (ctx)
+	  || (DECL_LANG_SPECIFIC (ctx)
+	      && DECL_DEFAULTED_FN (ctx))
+	  || (LAMBDA_FUNCTION_P (ctx)
+	      && LAMBDA_EXPR_REGEN_INFO (CLASSTYPE_LAMBDA_EXPR
+					 (DECL_CONTEXT (ctx)))))
+	/* It suffices to check shadowing only when actually parsing.
+	   So punt for clones, instantiations, defaulted functions and
+	   regenerated lambdas.  This optimization helps reduce lazy
+	   loading cascades with modules.  */
+	return NULL_TREE;
+    }
+
   tree old = NULL_TREE;
   cp_binding_level *old_scope = NULL;
   if (cxx_binding *binding = outer_binding (DECL_NAME (decl), NULL, true))

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2024-02-13 19:27 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-13 19:27 [gcc r14-8962] c++/modules: reduce lazy loading recursion Patrick Palka

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