On Thu, 13 Jun 2024 at 19:57, Jonathan Wakely <jwak...@redhat.com> wrote: > > On Thu, 13 Jun 2024 at 18:40, François Dumont <frs.dum...@gmail.com> wrote: > > > > Hi > > > > Following your recent change here: > > > > https://gcc.gnu.org/pipermail/libstdc++/2024-June/058998.html > > > > I think we also need to fix the memset at bucket allocation level. > > > > I did it trying also to be more fancy pointer friendly by running > > __uninitialized_default_n_a on the allocator returned pointer rather > > than on the __to_address result. I wonder if an __uninitialized_fill_n_a > > would have been better ? Doing so I also had to call std::_Destroy on > > deallocation. Let me know if it is too early. > > You don't need the RAII guard. Initializing Alloc::pointer isn't > allowed to throw exceptions: > > "An allocator type X shall meet the Cpp17CopyConstructible > requirements (Table 32). The XX::pointer, > XX::const_pointer, XX::void_pointer, and XX::const_void_pointer types > shall meet the Cpp17Nullable- > Pointer requirements (Table 36). No constructor, comparison operator > function, copy operation, move > operation, or swap operation on these pointer types shall exit via an > exception." > > And you should not pass the allocator to the __uninitialized_xxx call, > nor the _Destroy call. We don't want to use the allocator's > construct/destroy members for those pointers. They are not container > elements. > > I think either uninitialized_fill_n with nullptr or > __uninitialized_default_n is fine. Not the _a forms taking an > allocator though.
And I'd use _Destroy_n(_M_buckets, _M_bucket_count) > > > I also wonder if the compiler will be able to optimize it to a memset > > call ? I'm interested to work on it if you confirm that it won't. > > It will do whatever is fastest, which might be memset or might be > vectorized code to zero it out (which is probably what libc memset > does too). > > > > > libstdc++: Do not use memset in _Hashtable buckets allocation > > > > Using memset is incorrect if the __bucket_ptr type is non-trivial, or > > does not use an all-zero bit pattern for its null value. > > > > Replace the use of memset with std::__uinitialized_default_n_a to set the > > pointers to nullptr. Doing so and corresponding std::_Destroy when > > deallocating > > buckets. > > > > libstdc++-v3/ChangeLog: > > > > * include/bits/hashtable_policy.h > > (_Hashtable_alloc::_M_allocate_buckets): Do not use memset to zero > > out bucket pointers. > > (_Hashtable_alloc::_M_deallocate_buckets): Add destroy of buckets. > > > > > > I hope you won't ask for copy rights on the changelog entry :-) > > > > Tested under Linux x64, ok to commit ? > > > > François
