| Message ID | 20240209215001.1196422-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] c++/modules: reduce lazy loading recursion | expand |
On 2/9/24 16:50, Patrick Palka wrote: > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look > OK for trunk > > -- >8 -- > > It turns out that with modules we can call mangle_decl recursively, > which is a problem 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 quite recursive > ultimately due to the name lookups from check_local_shadow, which may > perform lazy loading, which cause us to instantiate/clone things, which > end up calling check_local_shadow. This patch fixes 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 definitely not actual parsing. > --- > 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 1d37e5a52b9..4b1f9391fee 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 e58f3b5cb4d..ca5ba87bc76 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))))) Maybe these tests should be factored out in case other places want to check the same condition? OK either way. > + /* It suffices to check shadowing only when actually parsing. > + So punt for clones, instantiations, defaulted functions and > + regenerated lambdas. This optimization helps 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))
diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc index 1d37e5a52b9..4b1f9391fee 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 e58f3b5cb4d..ca5ba87bc76 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 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))