On 4/4/24 07:27, Nathaniel Shead wrote:
On Wed, Apr 03, 2024 at 11:18:01AM -0400, Jason Merrill wrote:
On 4/2/24 20:57, Nathaniel Shead wrote:
On Tue, Apr 02, 2024 at 01:18:17PM -0400, Jason Merrill wrote:
On 3/28/24 23:21, Nathaniel Shead wrote:
- && !(modules_p () && DECL_DECLARED_INLINE_P (fn)))
+ && !(modules_p ()
+ && (DECL_DECLARED_INLINE_P (fn)
+ || DECL_TEMPLATE_INSTANTIATION (fn))))
How about using vague_linkage_p?
Right, of course. How about this?
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
-- >8 --
A template instantiation still needs to have its DECL_SAVED_TREE so that
its definition is emitted into the CMI. This way it can be emitted in
the object file of any importers that use it, in case it doesn't end up
getting emitted in this TU.
PR c++/104040
gcc/cp/ChangeLog:
* semantics.cc (expand_or_defer_fn_1): Keep DECL_SAVED_TREE for
all vague linkage functions.
gcc/testsuite/ChangeLog:
* g++.dg/modules/pr104040_a.C: New test.
* g++.dg/modules/pr104040_b.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
Reviewed-by: Jason Merrill <ja...@redhat.com>
---
gcc/cp/semantics.cc | 5 +++--
gcc/testsuite/g++.dg/modules/pr104040_a.C | 14 ++++++++++++++
gcc/testsuite/g++.dg/modules/pr104040_b.C | 8 ++++++++
3 files changed, 25 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/pr104040_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/pr104040_b.C
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index adb1ba48d29..03800a20b26 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -5033,9 +5033,10 @@ expand_or_defer_fn_1 (tree fn)
/* We don't want to process FN again, so pretend we've written
it out, even though we haven't. */
TREE_ASM_WRITTEN (fn) = 1;
- /* If this is a constexpr function, keep DECL_SAVED_TREE. */
+ /* If this is a constexpr function, or the body might need to be
+ exported from a module CMI, keep DECL_SAVED_TREE. */
if (!DECL_DECLARED_CONSTEXPR_P (fn)
- && !(modules_p () && DECL_DECLARED_INLINE_P (fn)))
+ && !(modules_p () && vague_linkage_p (fn)))
Also, how about module_maybe_has_cmi_p? OK with that change.
Using 'module_maybe_has_cmi_p' doesn't seem to work. This is for two
reasons, one of them fixable and one of them not (easily):
- It seems that header modules don't count for 'module_maybe_has_cmi_p';
I didn't notice this initially, and maybe they should for the
no-linkage decls too?
I think so; they could similarly be referred to by an importer.
But even accounting for this,
- For some reason only clearing it if the module might have a CMI causes
crashes in importers for some testcases. I'm not 100% sure why yet,
but I suspect it might be some duplicate-decls thing where the type
inconsistently has DECL_SAVED_TREE applied, since this is also called
on streamed-in declarations.
Clearing if the module might have a CMI sounds backwards, I'd expect
that to be the case where we want to leave it alone. Is that the
problem, or just a typo?
Out of interest, what was the reason that it was cleared at all in the
first place? I wasn't able to find anything with git blame; is it just
for performance reasons in avoiding excess lowering later?
That change goes back to the LTO merge, I believe it was to reduce
unnecessary LTO streaming.
But now that I think about it some more, I don't see why handling
modules specially here is necessary at all; the point of this code is
that after we build the destructor clones, the DECL_SAVED_TREE of the
cloned function is no longer useful. Why would modules care about the
maybe-in-charge function?
Jason
