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. > 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
