From patchwork Thu Jun 13 17:39:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Fran=C3=A7ois_Dumont?= X-Patchwork-Id: 1947523 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=S0xaIfUm; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4W0V7S52HHz1ydW for ; Fri, 14 Jun 2024 03:40:51 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6A261388215F for ; Thu, 13 Jun 2024 17:40:45 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) by sourceware.org (Postfix) with ESMTPS id 7F3EA3882072; Thu, 13 Jun 2024 17:39:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7F3EA3882072 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 7F3EA3882072 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::335 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718300391; cv=none; b=IlLguW32vj5Nu/ljpLZ2kHk6fatV4dTtGQHp2LyKOlQuD1FFJkUQry5JobH6APSJC3HUFT3yLoAsfikQ9NXCCnxyA4rsocj0RKQAZEn6RoQZTYC74O4t7J6M5xcJDdvWvIB26g5qb2mVwJRz0x8qcH8dyp/1baq/5WCCt6qUuaM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718300391; c=relaxed/simple; bh=N+Xj5eJFZ1VJju2sF9NWwrYflwtbYY6VyGdHOAFQmFQ=; h=DKIM-Signature:Message-ID:Date:MIME-Version:To:From:Subject; b=V88dXvixM5hH95G0dewDQWl4gdQsiVud9ABaW5w1zTsaHYrHL6GxDRaZk5laFHruBVNO9EQGG3+UDskWLGx8ZL14gocc1Dkx848iU8n5S4fS8WZqDJVoMRb+VBkLKnU+7lwb0w2WOMNqHWHR4L4SJJV7CatHnVKucurntn52gtg= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wm1-x335.google.com with SMTP id 5b1f17b1804b1-421d32fda86so14891995e9.0; Thu, 13 Jun 2024 10:39:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1718300388; x=1718905188; darn=gcc.gnu.org; h=subject:from:cc:to:content-language:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=23mTUZw4/Kzm+zi2/slIKAK5hEp1/yQCXaNQimZVrt8=; b=S0xaIfUmFImJir7wvjTTTYtl3U7Lzb27CYuttiR2hssQ+y0z+vCdYd6LNO0K3FLxYq w/YaqvVGtskj1vCJpbwsm1qI7fnm2quJZdcHJ1j15ELP84EpMye38D0eRnt/Iew+sXkV RtPjZYcpW3YQUF9aLZ8xvar3XMYOKHJU0lZjr+8VaXghg4vt0JVb3oa2fVMkg/F2Fwlj gW50EJqupR2WSyexCQ+lqrdwFtY3qVf+b3FHYsZqkPmzF6/rWmoMBcENRjAA/LxtjJfa 7lUDDVzp/Dvtm4wb1xF6AEIf7sYbaGkTE8gmTWLYN9NyJ52Yrdv4d34kBwTP2fhgB+GE lOng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718300388; x=1718905188; h=subject:from:cc:to:content-language:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=23mTUZw4/Kzm+zi2/slIKAK5hEp1/yQCXaNQimZVrt8=; b=BbsKGjZWDys4aMpubXx7IGgj1zrtSZawrwr3qO9KGKF35NzIyy4BPmhrJlsrIrtASO eIde6yuzHBOx5lX6RxRf9Rbsbr0W4Yr+yUlW+Ol7MRxIPiiaUmTIkzPXDYSCGtEvsWhi Yp0WQ1g4AGttfCf4yiqTZonFPdf0Vx6TuGWar4X0qazgFPNdoMNATXk1RbbEs4mFHESH Yt5Ti7lDkUwv3tZetvS8NbL6f4FO0BkcyQhIDmiAg2BoU0BlmrMRegd5aPhFIbSeG09e fo0mbTUbdcxg08MozEjfkbzdmSQblP/kqoJ4ThJ9Kk107a19XYOnstuXC4bl/WRK/boh oQtA== X-Gm-Message-State: AOJu0YxzPHVNHvvWoJBjsnbHVYkmtkmQuMzp6V4VFghGHnfVn24hMNvz Fgel9HaGJwREyV6l9iPyAR2vTjwQDmGrie54Fs/YJug1I6R4vrEQe/XzAw== X-Google-Smtp-Source: AGHT+IH9ET+PTkantZQ1kwYTW3kcoe41JZYG3tUASaBLWaW/D3NOzUpJtbFEYtrzwo+DLsX1GxSd5Q== X-Received: by 2002:a05:600c:4f49:b0:422:6755:17de with SMTP id 5b1f17b1804b1-4230485678cmr4572175e9.41.1718300387542; Thu, 13 Jun 2024 10:39:47 -0700 (PDT) Received: from [10.56.1.9] ([89.207.171.133]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-422870e9676sm68548005e9.24.2024.06.13.10.39.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 13 Jun 2024 10:39:46 -0700 (PDT) Message-ID: <879ddf66-abed-49b5-bbdf-2628696223f8@gmail.com> Date: Thu, 13 Jun 2024 19:39:42 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: libstdc++ Cc: gcc-patches From: =?utf-8?q?Fran=C3=A7ois_Dumont?= Subject: [PATCH] libstdc++: Do not use memset _Hashtable buckets allocation X-Spam-Status: No, score=-9.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_SBL_CSS, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org 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. 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. 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 diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h index 26def24f24e..6456c53e8b8 100644 --- a/libstdc++-v3/include/bits/hashtable_policy.h +++ b/libstdc++-v3/include/bits/hashtable_policy.h @@ -33,8 +33,9 @@ #include // for std::tuple, std::forward_as_tuple #include // for __is_fast_hash -#include // for std::min, std::is_permutation. +#include // for std::min, std::is_permutation #include // for std::pair +#include // for __uninitialized_default_n_a #include // for __gnu_cxx::__aligned_buffer #include // for std::__alloc_rebind #include // for __gnu_cxx::__int_traits @@ -2068,12 +2069,39 @@ namespace __detail _Hashtable_alloc<_NodeAlloc>::_M_allocate_buckets(std::size_t __bkt_count) -> __buckets_ptr { - __buckets_alloc_type __alloc(_M_node_allocator()); + // RAII guard for allocated storage. + struct _Guard_alloc + { + __buckets_ptr _M_storage; // Storage to deallocate + std::size_t _M_len; + __buckets_alloc_type& _M_alloc; + + _Guard_alloc(__buckets_ptr __s, std::size_t __l, + __buckets_alloc_type& __alloc) + : _M_storage(__s), _M_len(__l), _M_alloc(__alloc) + { } + _Guard_alloc(const _Guard_alloc&) = delete; + + ~_Guard_alloc() + { + if (_M_storage) + __buckets_alloc_traits::deallocate(_M_alloc, _M_storage, _M_len); + } + + __buckets_ptr + _M_release() + { + auto __res = _M_storage; + _M_storage = nullptr; + return __res; + } + }; + __buckets_alloc_type __alloc(_M_node_allocator()); auto __ptr = __buckets_alloc_traits::allocate(__alloc, __bkt_count); - __buckets_ptr __p = std::__to_address(__ptr); - __builtin_memset(__p, 0, __bkt_count * sizeof(__node_base_ptr)); - return __p; + _Guard_alloc __guard(__ptr, __bkt_count, __alloc); + std::__uninitialized_default_n_a(__ptr, __bkt_count, __alloc); + return std::__to_address(__guard._M_release()); } template @@ -2085,6 +2113,7 @@ namespace __detail typedef typename __buckets_alloc_traits::pointer _Ptr; auto __ptr = std::pointer_traits<_Ptr>::pointer_to(*__bkts); __buckets_alloc_type __alloc(_M_node_allocator()); + std::_Destroy(__ptr, __ptr + __bkt_count, __alloc); __buckets_alloc_traits::deallocate(__alloc, __ptr, __bkt_count); }