On Wed, 27 May 2026, 19:12 Nathan Myers, <[email protected]> wrote:
> Changes in v4:
> - Simplify calculation.
> - Drop use of unreachable(), as compilers seem unable to use it.
> - Distinguish objects of size 8 or more, where __bonus can be only
> 0 or 1, because compilers do not recognize it.
> - Test more thoroughly.
>
> Changes in v3:
> - Calculate extra allocations with no attempt to force 8-bit
> division, which pessimized everything except x86.
> - Add test to verify allocate_at_least change.
> - Also fix vector insert_vs_emplace.cc as run built with -m32.
>
> (Please ignore v2.)
>
> allocate_at_least is rounding up the allocation request size to
> its default alignment, which may be more than an integral
> multiple of the object size requested. When the memory is freed,
> what the container reports it is freeing differs from the amount
> that was allocated. This patch rounds the request size back down
> to what will be reported to the caller.
>
> The algorithm to compute the allocation is altered in response
> to findings on godbolt.org, which indicate dropping to uint8 to
> perform the division is a pessimization everywhere other than
> x86. Most targets implement the altered code with multiplication,
> instead.
>
> In addition, the remaining -m32 test that failed under the new
> allocation method is fixed.
>
Thanks for fixing this test.
> Tested on x86 -m64/-m32.
>
> libstdc++-v3/ChangeLog:
> * include/bits/new_allocator.h (allocate_at_least): Reduce
> allocation to match what is reported.
> * testsuite/20_util/allocator/allocate_at_least2.cc: New tests.
> * testsuite/23_containers/vector/modifiers/insert_vs_emplace.cc:
> Fix for -m32 and new allocation results.
---
> libstdc++-v3/include/bits/new_allocator.h | 28 ++++++----
> .../20_util/allocator/allocate_at_least2.cc | 55 +++++++++++++++++++
> .../vector/modifiers/insert_vs_emplace.cc | 48 ++++++++++------
> 3 files changed, 103 insertions(+), 28 deletions(-)
> create mode 100644
> libstdc++-v3/testsuite/20_util/allocator/allocate_at_least2.cc
>
> diff --git a/libstdc++-v3/include/bits/new_allocator.h
> b/libstdc++-v3/include/bits/new_allocator.h
> index 4524355a4a0..318365d8f0a 100644
> --- a/libstdc++-v3/include/bits/new_allocator.h
> +++ b/libstdc++-v3/include/bits/new_allocator.h
> @@ -184,17 +184,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> if constexpr ( sizeof(_Tp) < __STDCPP_DEFAULT_NEW_ALIGNMENT__)
> {
> _S_check_allocation_limit(__n);
> - const size_t __need = __n * sizeof(_Tp);
> + size_t __ask = __n * sizeof(_Tp);
>
Now that we no longer have "need" and "ask", would it make sense to just
call this __bytes instead of __ask?
We have n for the count of objects, and bytes is the count of bytes.
> const size_t __mask = __STDCPP_DEFAULT_NEW_ALIGNMENT__ - 1;
> - size_t __ask = (__need + __mask) & ~__mask;
> - // Avoid rounding up to and asking for 2^63 bytes
> (PR108377):
> - __ask -= __ask >> (__SIZE_WIDTH__ - 1);
> - auto* __p =
> static_cast<_Tp*>(_GLIBCXX_OPERATOR_NEW(__ask));
> - using _U8 = const unsigned char;
> - static_assert(sizeof(_Tp) <= ~_U8());
> - // Use 8-bit division for minimal latency:
> - _U8 __spare = __ask - __need, __size = sizeof(_Tp);
> - return { __p , __n + __spare / __size };
> + size_t __max = (__ask + __mask) & ~__mask;
> + // Avoid seeming to ask for 2^63 bytes (PR108377):
> + __max -= __max >> (__SIZE_WIDTH__ - 1);
> + auto __spare = static_cast<unsigned>(__max - __ask);
> + if constexpr (sizeof(_Tp) < (__mask + 1) / 2)
> + {
> + auto __bonus = __spare / sizeof(_Tp);
> + __n += __bonus;
> + __ask += __bonus * sizeof(_Tp);
> + }
> + else if (sizeof(_Tp) <= __spare)
> + {
> + __n += 1;
> + __ask += sizeof(_Tp);
> + }
> + void* __p = _GLIBCXX_OPERATOR_NEW(__ask);
> + return { static_cast<_Tp*>(__p), __n };
> }
> }
> return { allocate(__n), __n };
> diff --git
> a/libstdc++-v3/testsuite/20_util/allocator/allocate_at_least2.cc
> b/libstdc++-v3/testsuite/20_util/allocator/allocate_at_least2.cc
> new file mode 100644
> index 00000000000..e429bef9bc4
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/20_util/allocator/allocate_at_least2.cc
> @@ -0,0 +1,55 @@
> +// { dg-do run { target c++23 } }
> +
> +#include <memory>
>
This should include <stdlib.h> for ::malloc and ::free.
+#include <testsuite_hooks.h>
> +
> +std::size_t gn = 0;
> +void* operator new(std::size_t n)
> +{
> + gn = n;
> + return ::malloc(n);
> +}
> +
> +void operator delete(void* p) noexcept
> +{ ::free(p); }
> +
> +void operator delete(void* p, std::size_t n) noexcept
> +{ (void)n; ::free(p); }
>
Any reason to not just omit the second parameter's name?
void operator delete(void* p, std::size_t) noexcept
{ ::free(p); }
This overload should be guarded with __cpp_sized_deallocation in case the
test is run with -fno-sized-deallocation
And that's made me realize that __new_allocator::allocate_at_least will be
ill-formed if somebody uses -std=gnu++26 -fno-aligned-new, because
the __STDCPP_DEFAULT_NEW_ALIGNMENT__ macro won't be defined.
I think the whole `if ! consteval` block can just be guarded by #if
__cpp_aligned_new so we just return { allocate(n), n } if -fno-aligned-new
is used, and this test needs adjusting so that 'count' is just 'n' when
__cpp_aligned_new is not defined.
+
> +template <unsigned size, unsigned n>
> +void test_nm()
> +{
> + struct A { char a[size]; };
> + std::allocator<A> alloc;
> + using alloc_traits = std::allocator_traits<std::allocator<A>>;
> + auto [p, m] = alloc_traits::allocate_at_least(alloc, n);
> + unsigned mod = __STDCPP_DEFAULT_NEW_ALIGNMENT__;
> + unsigned max = ((n * size) + mod - 1) & ~(mod - 1);
> + unsigned count = max / sizeof(A);
> + VERIFY(m == count);
> + VERIFY(gn == count * sizeof(A));
> +}
> +
> +void test()
> +{ // m gn
> + test_nm<1,3>(); // 16 16
> + test_nm<2,3>(); // 8 16
> + test_nm<3,3>(); // 5 15
> + test_nm<4,3>(); // 4 16
> + test_nm<5,3>(); // 3 15
> + test_nm<6,3>(); // 5 30
> + test_nm<7,3>(); // 4 28
> + test_nm<8,3>(); // 4 32
> + test_nm<9,3>(); // 3 27
> + test_nm<10,3>(); // 3 30
> + test_nm<11,3>(); // 4 44
> + test_nm<12,3>(); // 4 48
> + test_nm<13,3>(); // 3 39
> + test_nm<14,3>(); // 3 42
> + test_nm<15,3>(); // 3 45
> +}
> +
> +int main()
> +{
> + test();
> +}
> diff --git
> a/libstdc++-v3/testsuite/23_containers/vector/modifiers/insert_vs_emplace.cc
> b/libstdc++-v3/testsuite/23_containers/vector/modifiers/insert_vs_emplace.cc
> index 187e433d9d3..d226490d6f3 100644
> ---
> a/libstdc++-v3/testsuite/23_containers/vector/modifiers/insert_vs_emplace.cc
> +++
> b/libstdc++-v3/testsuite/23_containers/vector/modifiers/insert_vs_emplace.cc
> @@ -223,11 +223,12 @@ test03()
> void
> test04()
> {
> - const X::special expected{ 0, 3, 1, 0, 3, 0 };
> + const X::special expected{ 0, 4, 1, 0, 4, 0 };
> X::special ins, emp;
> {
> std::vector<X> v;
> - v.reserve(3);
> + v.reserve(4);
> + v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> @@ -241,7 +242,8 @@ test04()
> }
> {
> std::vector<X> v;
> - v.reserve(3);
> + v.reserve(4);
> + v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> @@ -261,11 +263,12 @@ test04()
> void
> test05()
> {
> - const X::special expected{ 0, 3, 0, 0, 4, 0 };
> + const X::special expected{ 0, 4, 0, 0, 5, 0 };
> X::special ins, emp;
> {
> std::vector<X> v;
> - v.reserve(3);
> + v.reserve(4);
> + v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> @@ -279,7 +282,8 @@ test05()
> }
> {
> std::vector<X> v;
> - v.reserve(3);
> + v.reserve(4);
> + v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> @@ -299,11 +303,12 @@ test05()
> void
> test06()
> {
> - const X::special expected{ 1, 4, 0, 0, 4, 0 };
> + const X::special expected{ 1, 5, 0, 0, 5, 0 };
> X::special ins, emp;
> {
> std::vector<X> v;
> - v.reserve(3);
> + v.reserve(4);
> + v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> @@ -316,7 +321,8 @@ test06()
> }
> {
> std::vector<X> v;
> - v.reserve(3);
> + v.reserve(4);
> + v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> @@ -447,11 +453,12 @@ test09()
> void
> test10()
> {
> - const X::special expected{ 0, 3, 1, 0, 3, 0 };
> + const X::special expected{ 0, 4, 1, 0, 4, 0 };
> X::special ins, emp;
> {
> std::vector<X> v;
> - v.reserve(3);
> + v.reserve(4);
> + v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> @@ -465,7 +472,8 @@ test10()
> }
> {
> std::vector<X> v;
> - v.reserve(3);
> + v.reserve(4);
> + v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> @@ -485,11 +493,12 @@ test10()
> void
> test11()
> {
> - const X::special expected{ 0, 3, 0, 0, 4, 0 };
> + const X::special expected{ 0, 4, 0, 0, 5, 0 };
> X::special ins, emp;
> {
> std::vector<X> v;
> - v.reserve(3);
> + v.reserve(4);
> + v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> @@ -503,7 +512,8 @@ test11()
> }
> {
> std::vector<X> v;
> - v.reserve(3);
> + v.reserve(4);
> + v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> @@ -523,11 +533,12 @@ test11()
> void
> test12()
> {
> - const X::special expected{ 1, 4, 0, 0, 4, 0 };
> + const X::special expected{ 1, 5, 0, 0, 5, 0 };
> X::special ins, emp;
> {
> std::vector<X> v;
> - v.reserve(3);
> + v.reserve(4);
> + v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> @@ -540,7 +551,8 @@ test12()
> }
> {
> std::vector<X> v;
> - v.reserve(3);
> + v.reserve(4);
> + v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> v.push_back(X(0,0));
> --
> 2.54.0
>
>
