On 4/24/24 09:16, Jakub Jelinek wrote:
On Wed, Apr 24, 2024 at 10:16:05AM +0100, Jonathan Wakely wrote:
That fixes the testcases too, but seems to regress
+FAIL: libstdc++-abi/abi_check
There are explicit instantiation definitions that should instantiate
those types:
src/c++17/fs_dir.cc:template class std::__shared_ptr<fs::_Dir>;
src/c++17/fs_dir.cc:template class
std::__shared_ptr<fs::recursive_directory_iterator::_Dir_stack>;
src/c++17/fs_path.cc:template class std::__shared_ptr<const
fs::filesystem_error::_Impl>;
So the missing symbols should be present in cow-fs_dir.o and cow-fs_path.o
So this boils down to (cvise reduced):
namespace __gnu_cxx { enum _Lock_policy { _S_single, _S_mutex, _S_atomic }
const __default_lock_policy = _S_atomic; }
namespace std {
using __gnu_cxx::__default_lock_policy;
using __gnu_cxx::_Lock_policy;
template <typename, _Lock_policy = __default_lock_policy> struct __shared_ptr {
constexpr __shared_ptr() {} };
namespace filesystem {
struct _Dir;
struct directory_iterator { __shared_ptr<_Dir> _M_dir; };
void end() { directory_iterator(); } }
extern template class __shared_ptr<filesystem::_Dir>;
}
namespace fs = std::filesystem;
template class std::__shared_ptr<fs::_Dir>;
at -O2, previously it used to emit
_ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev
_ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE1EEC2Ev
but now no longer does with the yesterday posted PR113208 patch.
The following updated patch fixes that by calling note_vague_linkage_fn for
the cdtor clones from maybe_clone_body if the flags suggest that the
maybe-in-charge cdtor had tentative_decl_linkage -> note_vague_linkage_fn
called too. And then I've noticed that in some cases the updated comdat
group set by maybe_clone_body (*C5* or *D5*) was then overwritten again by
maybe_make_one_only. So the patch tweaks cxx_comdat_group, so that when
some comdat group has been chosen already it doesn't try to use some
different one.
Bootstrapped/regtested on x86_64-linux and i686-linux, this one doesn't
regress anything unlike the earlier patch.
2024-04-24 Jakub Jelinek <ja...@redhat.com>
PR lto/113208
* decl2.cc (tentative_decl_linkage): Use comdat_linkage also
for implicit instantiations of maybe in charge ctors/dtors
if -fimplicit-templates or -fimplicit-inline-templates and
-fweak and target supports aliases.
* optimize.cc (maybe_clone_body): Call note_vague_linkage_fn
on clone if fn has DECL_INTERFACE_KNOWN, DECL_NOT_REALLY_EXTERN
and DECL_DEFER_OUTPUT flags set.
* decl.cc (cxx_comdat_group): For DECL_CLONED_FUNCTION_P
functions if SUPPORTS_ONE_ONLY return DECL_COMDAT_GROUP if already
set.
* g++.dg/abi/comdat2.C: New test.
* g++.dg/abi/comdat3.C: New test.
* g++.dg/lto/pr113208_0.C: New test.
* g++.dg/lto/pr113208_1.C: New file.
* g++.dg/lto/pr113208.h: New file.
--- gcc/cp/decl2.cc.jj 2024-04-23 14:49:41.933186265 +0200
+++ gcc/cp/decl2.cc 2024-04-24 15:17:09.043625729 +0200
@@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl)
to mark the functions at this point. */
if (DECL_DECLARED_INLINE_P (decl)
&& (!DECL_IMPLICIT_INSTANTIATION (decl)
- || DECL_DEFAULTED_FN (decl)))
+ || DECL_DEFAULTED_FN (decl)
+ /* For implicit instantiations of cdtors,
+ if import_export_decl would use comdat linkage,
+ make sure to use it right away, so that maybe_clone_body
+ can use aliases. See PR113208. */
+ || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)
+ && (flag_implicit_templates
+ || flag_implicit_inline_templates)
+ && flag_weak
+ && TARGET_SUPPORTS_ALIASES)))
It seems wrong to set DECL_INTERFACE_KNOWN for cdtors that might get an
explicit instantiation later, and likewise for comdat_linkage when
!flag_weak; instead of adding this condition to the if, how about adding
an else like
else if (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl))
/* For implicit instantiations of cdtors,
if import_export_decl would use comdat linkage,
make sure to use it right away, so that maybe_clone_body
can use aliases. See PR113208. */
maybe_make_one_only (decl);
?
Jason
