On 3/27/25 3:35 AM, Nathaniel Shead wrote:
Bootstrapped and regtested (so far just dg.exp and modules.exp) on
x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds?
Rather than updating copy_fndecl_with_name, we could also just fix
modules specifically by overwriting DECL_ABSTRACT_P before calling
build_cdtor_clones in trees_in::decl_value, or by forcing it to 0 for
DECL_MAYBE_IN_CHARGE_CDTOR during tree streaming, if you prefer, since
it'll always be set again by expand_or_defer_fn anyway.
-- >8 --
This patch makes some adjustments required to get a simple modules
testcase working with LTO. There are two main issues fixed.
Firstly, modules only streams the maybe-in-charge constructor, and any
clones are recreated on stream-in. These clones are copied from the
existing function decl and then adjusted. This caused issues because
the clones were getting incorrectly marked as abstract, since after
clones have been created (in the imported file) the maybe-in-charge decl
gets marked as abstract. So this patch just ensures that clones are
always created as non-abstract.
Sounds good.
The second issue is that we need to explicitly tell cgraph that explicit
instantiations need to be emitted, otherwise LTO will elide them (as
they don't necessarily appear to be used directly) and cause link
errors.
Makes sense. Maybe you want to check get_importer_interface ==
always_emit instead of specifically for explicit inst?
...except I see that it returns that value for internal decls, which
don't actually always need to be emitted; there seems to be a missing
distinction between "considered to be defined in this TU" and actually
"always emit".
Additionally, expand_or_defer_fn doesn't setup comdat groups
for explicit instantiations, so we need to do that here as well.
Hmm, that inconsistency seems worth fixing in expand_or_defer_fn, though
it's fine to leave that for later and just add a FIXME comment to your
change.
PR c++/118961
gcc/cp/ChangeLog:
* class.cc (copy_fndecl_with_name): Mark clones as non-abstract.
* module.cc (trees_in::read_var_def): Explicit instantiation
definitions of variables must be emitted, and are COMDAT.
(module_state::read_cluster): Likewise for functions.
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 2cded878c64..f9f48bb2421 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -12684,6 +12684,13 @@ trees_in::read_var_def (tree decl, tree maybe_template)
if (maybe_dup && DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P
(maybe_dup))
DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = true;
tentative_decl_linkage (decl);
Doesn't this handle the comdat for variables?
+ if (DECL_EXPLICIT_INSTANTIATION (decl)
+ && !DECL_EXTERNAL (decl))
+ {
+ mark_needed (decl);
+ if (TREE_PUBLIC (decl))
+ maybe_make_one_only (decl);
+ }
if (DECL_IMPLICIT_INSTANTIATION (decl)
|| (DECL_EXPLICIT_INSTANTIATION (decl)
&& !DECL_EXTERNAL (decl))
Jason
