libstdc++: Compile std::allocator instantiations as C++20
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Testing passed
|
Commit Message
I'm considering this late patch for gcc-14 to workaround an issue
discovered by a recent Clang change.
I'm not yet sure if Clang is right to require these symbols. It's not
really clear, because always_inline isn't part of the standard so it's
not clear how it should interact with explicit instantiations and
modules. Exporting these four extra symbols doesn't hurt, even if Clang
ends up reverting or revising its change that requires them.
Another way to fix it would be to suppress the explicit instantiation
declarations in <bits/allocator.h> for C++20, so that the compiler
always instantiates them implicitly as needed. We do similar things for
the explicit instantiations of std::string etc. so that new member
functions that aren't in the .so are implicitly instantiated as needed.
That would look like this instead:
Comments
On Thu, 11 Apr 2024 at 20:22, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> I'm considering this late patch for gcc-14 to workaround an issue
> discovered by a recent Clang change.
>
> I'm not yet sure if Clang is right to require these symbols. It's not
> really clear, because always_inline isn't part of the standard so it's
> not clear how it should interact with explicit instantiations and
> modules. Exporting these four extra symbols doesn't hurt, even if Clang
> ends up reverting or revising its change that requires them.
>
> Another way to fix it would be to suppress the explicit instantiation
> declarations in <bits/allocator.h> for C++20, so that the compiler
> always instantiates them implicitly as needed. We do similar things for
> the explicit instantiations of std::string etc. so that new member
> functions that aren't in the .so are implicitly instantiated as needed.
>
> That would look like this instead:
>
> --- a/libstdc++-v3/include/bits/allocator.h
> +++ b/libstdc++-v3/include/bits/allocator.h
> @@ -281,7 +281,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> // Inhibit implicit instantiations for required instantiations,
> // which are defined via explicit instantiations elsewhere.
> -#if _GLIBCXX_EXTERN_TEMPLATE
> +#if _GLIBCXX_EXTERN_TEMPLATE && __cplusplus <= 201703L
> extern template class allocator<char>;
> extern template class allocator<wchar_t>;
> #endif
>
> But we might want to export the new functions from the library
> eventually anyway, so doing it now (before Clang 19 is released) might
> be the best option.
>
> Thoughts?
I think the symbol export is a fine solution. Both of these solutions
work, so I don't have a strong preference,
I have a minor preference for not suppressing explicit instantiations
that are otherwise already there,
but that is indeed not a strong preference.
> On 11 Apr 2024, at 18:33, Ville Voutilainen <ville.voutilainen@gmail.com> wrote:
>
> On Thu, 11 Apr 2024 at 20:22, Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>> I'm considering this late patch for gcc-14 to workaround an issue
>> discovered by a recent Clang change.
>>
>> I'm not yet sure if Clang is right to require these symbols. It's not
>> really clear, because always_inline isn't part of the standard so it's
>> not clear how it should interact with explicit instantiations and
>> modules. Exporting these four extra symbols doesn't hurt, even if Clang
>> ends up reverting or revising its change that requires them.
>>
>> Another way to fix it would be to suppress the explicit instantiation
>> declarations in <bits/allocator.h> for C++20, so that the compiler
>> always instantiates them implicitly as needed. We do similar things for
>> the explicit instantiations of std::string etc. so that new member
>> functions that aren't in the .so are implicitly instantiated as needed.
>>
>> That would look like this instead:
>>
>> --- a/libstdc++-v3/include/bits/allocator.h
>> +++ b/libstdc++-v3/include/bits/allocator.h
>> @@ -281,7 +281,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>
>> // Inhibit implicit instantiations for required instantiations,
>> // which are defined via explicit instantiations elsewhere.
>> -#if _GLIBCXX_EXTERN_TEMPLATE
>> +#if _GLIBCXX_EXTERN_TEMPLATE && __cplusplus <= 201703L
>> extern template class allocator<char>;
>> extern template class allocator<wchar_t>;
>> #endif
>>
>> But we might want to export the new functions from the library
>> eventually anyway, so doing it now (before Clang 19 is released) might
>> be the best option.
I think clang-19 will branch after gcc-14 (so the urgency is more on the GCC
side - or making a decision to back out of the clang change.
>>
>> Thoughts?
>
> I think the symbol export is a fine solution. Both of these solutions
> work, so I don't have a strong preference,
> I have a minor preference for not suppressing explicit instantiations
> that are otherwise already there,
> but that is indeed not a strong preference.
In the case of interaction between modules and items not identified in
the std, the only solution is to get agreement between the ‘vendors’.
If we emit the symbols, then that gets baked into the libstdc++ ABI, right?
Is there an alternate solution that we can propose? (there is a modules
implementer’s group that meets bi-weekly, including next Tuesday -
this includes representation from GCC, clang, MSVC and usually some
of the larger players like google… it could be a topic of discussion if
there is some tidier proposal we could make.
Iain
@@ -281,7 +281,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// Inhibit implicit instantiations for required instantiations,
// which are defined via explicit instantiations elsewhere.
-#if _GLIBCXX_EXTERN_TEMPLATE
+#if _GLIBCXX_EXTERN_TEMPLATE && __cplusplus <= 201703L
extern template class allocator<char>;
extern template class allocator<wchar_t>;
#endif
But we might want to export the new functions from the library
eventually anyway, so doing it now (before Clang 19 is released) might
be the best option.
Thoughts?
-- >8 --
This ensures that the new std::allocator::allocate and
std::allocator::deallocate functions are included in the explicit
instantiation definitions. They're only defined for C++20 and later
(because they are needed to support constexpr dynamic allocation) so
were not being instantiated when those files were compiled as C++98.
This isn't needed for GCC because those functions are marked
'always_inline' and so no extern symbols are needed. But a recent Clang
change has meant that always_inline functions do not get exported from
modules, and so the extern symbols (which were not previously being
instantiated or exported) are needed by Clang. See the Clang issue
https://github.com/llvm/llvm-project/issues/86893 for details.
The allocator-inst.cc file is changed to include <bits/allocator.h>
instead of all of <memory>, because otherwise it indirectly includes
<bits/basic_string.h> and the implicit instantiations of std::string and
std::wstring also instantiate std::allocator<char> and
std::allocator<wchar_t>, which for some reason suppresses the explicit
instantiation definitions of the default constructors in this file.
Just including <bits/allocator.h> avoids that problem.
libstdc++-v3/ChangeLog:
* config/abi/pre/gnu.ver: Export new symbols.
* src/c++20/Makefile.am: Add allocator-inst.o.
* src/c++20/Makefile.in: Regenerate.
* src/c++98/allocator-inst.cc: Move to...
* src/c++20/allocator-inst.cc: ...here.
* src/c++98/Makefile.am: Remove allocator-inst.o.
* src/c++98/Makefile.in: Regenerate.
---
libstdc++-v3/config/abi/pre/gnu.ver | 4 ++++
libstdc++-v3/src/c++20/Makefile.am | 1 +
libstdc++-v3/src/c++20/Makefile.in | 4 +++-
libstdc++-v3/src/{c++98 => c++20}/allocator-inst.cc | 2 +-
libstdc++-v3/src/c++98/Makefile.am | 1 -
libstdc++-v3/src/c++98/Makefile.in | 6 ++----
6 files changed, 11 insertions(+), 7 deletions(-)
rename libstdc++-v3/src/{c++98 => c++20}/allocator-inst.cc (97%)
@@ -2526,6 +2526,10 @@ GLIBCXX_3.4.32 {
GLIBCXX_3.4.33 {
# std::basic_file<char>::native_handle()
_ZNKSt12__basic_fileIcE13native_handleEv;
+
+ # std::allocator<[cw]>::allocate and ::deallocate for C++20
+ _ZNSaI[cw]E8allocateE[jmy];
+ _ZNSaI[cw]E10deallocateEP[cw][jmy];
} GLIBCXX_3.4.32;
# Symbols in the support library (libsupc++) have their own tag.
@@ -30,6 +30,7 @@ headers =
if ENABLE_EXTERN_TEMPLATE
# XTEMPLATE_FLAGS = -fno-implicit-templates
inst_sources = \
+ allocator-inst.cc \
sstream-inst.cc
else
# XTEMPLATE_FLAGS =
@@ -122,7 +122,8 @@ CONFIG_CLEAN_VPATH_FILES =
LTLIBRARIES = $(noinst_LTLIBRARIES)
libc__20convenience_la_LIBADD =
am__objects_1 = tzdb.lo
-@ENABLE_EXTERN_TEMPLATE_TRUE@am__objects_2 = sstream-inst.lo
+@ENABLE_EXTERN_TEMPLATE_TRUE@am__objects_2 = allocator-inst.lo \
+@ENABLE_EXTERN_TEMPLATE_TRUE@ sstream-inst.lo
@GLIBCXX_HOSTED_TRUE@am_libc__20convenience_la_OBJECTS = \
@GLIBCXX_HOSTED_TRUE@ $(am__objects_1) $(am__objects_2)
libc__20convenience_la_OBJECTS = $(am_libc__20convenience_la_OBJECTS)
@@ -430,6 +431,7 @@ headers =
# XTEMPLATE_FLAGS = -fno-implicit-templates
@ENABLE_EXTERN_TEMPLATE_TRUE@inst_sources = \
+@ENABLE_EXTERN_TEMPLATE_TRUE@ allocator-inst.cc \
@ENABLE_EXTERN_TEMPLATE_TRUE@ sstream-inst.cc
sources = tzdb.cc
similarity index 97%
rename from libstdc++-v3/src/c++98/allocator-inst.cc
rename to libstdc++-v3/src/c++20/allocator-inst.cc
@@ -26,7 +26,7 @@
// ISO C++ 14882:
//
-#include <memory>
+#include <bits/allocator.h>
namespace std _GLIBCXX_VISIBILITY(default)
{
@@ -96,7 +96,6 @@ endif
if ENABLE_EXTERN_TEMPLATE
# XTEMPLATE_FLAGS = -fno-implicit-templates
inst_sources = \
- allocator-inst.cc \
concept-inst.cc \
ext-inst.cc \
misc-inst.cc
@@ -130,9 +130,8 @@ am__objects_3 = $(am__objects_2) codecvt_members.lo collate_members.lo \
messages_members.lo monetary_members.lo numeric_members.lo \
time_members.lo
am__objects_4 = c++locale.lo
-@ENABLE_EXTERN_TEMPLATE_TRUE@am__objects_5 = allocator-inst.lo \
-@ENABLE_EXTERN_TEMPLATE_TRUE@ concept-inst.lo ext-inst.lo \
-@ENABLE_EXTERN_TEMPLATE_TRUE@ misc-inst.lo
+@ENABLE_EXTERN_TEMPLATE_TRUE@am__objects_5 = concept-inst.lo \
+@ENABLE_EXTERN_TEMPLATE_TRUE@ ext-inst.lo misc-inst.lo
am__objects_6 = parallel_settings.lo
am__objects_7 = bitmap_allocator.lo pool_allocator.lo mt_allocator.lo \
codecvt.lo complex_io.lo globals_io.lo hash_tr1.lo \
@@ -478,7 +477,6 @@ host_sources_extra = \
# XTEMPLATE_FLAGS = -fno-implicit-templates
@ENABLE_EXTERN_TEMPLATE_TRUE@inst_sources = \
-@ENABLE_EXTERN_TEMPLATE_TRUE@ allocator-inst.cc \
@ENABLE_EXTERN_TEMPLATE_TRUE@ concept-inst.cc \
@ENABLE_EXTERN_TEMPLATE_TRUE@ ext-inst.cc \
@ENABLE_EXTERN_TEMPLATE_TRUE@ misc-inst.cc