From patchwork Thu Jun 1 15:09:58 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 1789161 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org 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=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=JSCMbge2; dkim-atps=neutral Received: from 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 (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4QX8hL0Ydhz20QB for ; Fri, 2 Jun 2023 01:10:26 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 0C860385B532 for ; Thu, 1 Jun 2023 15:10:24 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0C860385B532 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1685632224; bh=1n1Df1Nw/Tj9RN31cYyPV2w9cuWzgQmcBPncxPTOVIA=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=JSCMbge2vSNXQH5KHUGnXo6Ifnog6B8KPjq3PUTDqcb5TiFaL9OmlSQSjihQn/kHr 0uL1l09Cg5g5aPKDKK3OUi/yEYQ44x/QMfh2ShdlV2C4c9wPDxrAcJNG7FC0qPtRNA nZLI0N+bYRuBnqtPy6UoAVAIUxN6oMiwt7xmNh7E= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id CD0DE3857348 for ; Thu, 1 Jun 2023 15:10:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CD0DE3857348 Received: from mimecast-mx02.redhat.com (66.187.233.88 [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-591-P7ZFgfoCPG6RDRipcqpEwA-1; Thu, 01 Jun 2023 11:10:00 -0400 X-MC-Unique: P7ZFgfoCPG6RDRipcqpEwA-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id AEF3B85A5BD; Thu, 1 Jun 2023 15:09:59 +0000 (UTC) Received: from localhost (unknown [10.42.28.9]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6267940CFD45; Thu, 1 Jun 2023 15:09:59 +0000 (UTC) To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [committed] libstdc++: Fix code size regressions in std::vector [PR110060] Date: Thu, 1 Jun 2023 16:09:58 +0100 Message-Id: <20230601150958.268109-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, 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.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jonathan Wakely via Gcc-patches From: Jonathan Wakely Reply-To: Jonathan Wakely Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" Tested powerpc64le-linux. Pusshed to trunk. -- >8 -- My r14-1452-gfb409a15d9babc change to add optimization hints to std::vector causes regressions because it makes std::vector::size() and std::vector::capacity() too big to inline. That's the opposite of what I wanted, so revert the changes to those functions. To achieve the original aim of optimizing vec.assign(vec.size(), x) we can add a local optimization hint to _M_fill_assign, so that it doesn't affect all other uses of size() and capacity(). Additionally, add the same hint to the _M_assign_aux overload for forward iterators and add that to the testcase. It would be nice to similarly optimize: if (vec1.size() == vec2.size()) vec1 = vec2; but adding hints to operator=(const vector&) doesn't help. Presumably the relationships between the two sizes and two capacities are too complex to track effectively. libstdc++-v3/ChangeLog: PR libstdc++/110060 * include/bits/stl_vector.h (_Vector_base::_M_invariant): Remove. (vector::size, vector::capacity): Remove calls to _M_invariant. * include/bits/vector.tcc (vector::_M_fill_assign): Add optimization hint to reallocating path. (vector::_M_assign_aux(FwdIter, FwdIter, forward_iterator_tag)): Likewise. * testsuite/23_containers/vector/capacity/invariant.cc: Moved to... * testsuite/23_containers/vector/modifiers/assign/no_realloc.cc: ...here. Check assign(FwdIter, FwdIter) too. * testsuite/23_containers/vector/types/1.cc: Revert addition of -Wno-stringop-overread option. --- libstdc++-v3/include/bits/stl_vector.h | 23 +------------------ libstdc++-v3/include/bits/vector.tcc | 17 ++++++++++---- .../assign/no_realloc.cc} | 6 +++++ .../testsuite/23_containers/vector/types/1.cc | 2 +- 4 files changed, 20 insertions(+), 28 deletions(-) rename libstdc++-v3/testsuite/23_containers/vector/{capacity/invariant.cc => modifiers/assign/no_realloc.cc} (70%) diff --git a/libstdc++-v3/include/bits/stl_vector.h b/libstdc++-v3/include/bits/stl_vector.h index e593be443bc..70ced3d101f 100644 --- a/libstdc++-v3/include/bits/stl_vector.h +++ b/libstdc++-v3/include/bits/stl_vector.h @@ -389,23 +389,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER protected: - __attribute__((__always_inline__)) - _GLIBCXX20_CONSTEXPR void - _M_invariant() const - { -#if __OPTIMIZE__ - if (this->_M_impl._M_finish < this->_M_impl._M_start) - __builtin_unreachable(); - if (this->_M_impl._M_finish > this->_M_impl._M_end_of_storage) - __builtin_unreachable(); - - size_t __sz = this->_M_impl._M_finish - this->_M_impl._M_start; - size_t __cap = this->_M_impl._M_end_of_storage - this->_M_impl._M_start; - if (__sz > __cap) - __builtin_unreachable(); -#endif - } - _GLIBCXX20_CONSTEXPR void _M_create_storage(size_t __n) @@ -1005,10 +988,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR size_type size() const _GLIBCXX_NOEXCEPT - { - _Base::_M_invariant(); - return size_type(this->_M_impl._M_finish - this->_M_impl._M_start); - } + { return size_type(this->_M_impl._M_finish - this->_M_impl._M_start); } /** Returns the size() of the largest possible %vector. */ _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR @@ -1095,7 +1075,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER size_type capacity() const _GLIBCXX_NOEXCEPT { - _Base::_M_invariant(); return size_type(this->_M_impl._M_end_of_storage - this->_M_impl._M_start); } diff --git a/libstdc++-v3/include/bits/vector.tcc b/libstdc++-v3/include/bits/vector.tcc index d6fdea2dd01..acd11e2dc68 100644 --- a/libstdc++-v3/include/bits/vector.tcc +++ b/libstdc++-v3/include/bits/vector.tcc @@ -270,15 +270,18 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER vector<_Tp, _Alloc>:: _M_fill_assign(size_t __n, const value_type& __val) { + const size_type __sz = size(); if (__n > capacity()) { + if (__n <= __sz) + __builtin_unreachable(); vector __tmp(__n, __val, _M_get_Tp_allocator()); __tmp._M_impl._M_swap_data(this->_M_impl); } - else if (__n > size()) + else if (__n > __sz) { std::fill(begin(), end(), __val); - const size_type __add = __n - size(); + const size_type __add = __n - __sz; _GLIBCXX_ASAN_ANNOTATE_GROW(__add); this->_M_impl._M_finish = std::__uninitialized_fill_n_a(this->_M_impl._M_finish, @@ -316,10 +319,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER _M_assign_aux(_ForwardIterator __first, _ForwardIterator __last, std::forward_iterator_tag) { + const size_type __sz = size(); const size_type __len = std::distance(__first, __last); if (__len > capacity()) { + if (__len <= __sz) + __builtin_unreachable(); + _S_check_init_len(__len, _M_get_Tp_allocator()); pointer __tmp(_M_allocate_and_copy(__len, __first, __last)); std::_Destroy(this->_M_impl._M_start, this->_M_impl._M_finish, @@ -332,14 +339,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER this->_M_impl._M_finish = this->_M_impl._M_start + __len; this->_M_impl._M_end_of_storage = this->_M_impl._M_finish; } - else if (size() >= __len) + else if (__sz >= __len) _M_erase_at_end(std::copy(__first, __last, this->_M_impl._M_start)); else { _ForwardIterator __mid = __first; - std::advance(__mid, size()); + std::advance(__mid, __sz); std::copy(__first, __mid, this->_M_impl._M_start); - const size_type __attribute__((__unused__)) __n = __len - size(); + const size_type __attribute__((__unused__)) __n = __len - __sz; _GLIBCXX_ASAN_ANNOTATE_GROW(__n); this->_M_impl._M_finish = std::__uninitialized_copy_a(__mid, __last, diff --git a/libstdc++-v3/testsuite/23_containers/vector/capacity/invariant.cc b/libstdc++-v3/testsuite/23_containers/vector/modifiers/assign/no_realloc.cc similarity index 70% rename from libstdc++-v3/testsuite/23_containers/vector/capacity/invariant.cc rename to libstdc++-v3/testsuite/23_containers/vector/modifiers/assign/no_realloc.cc index d68db694add..659f18dba56 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/capacity/invariant.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/modifiers/assign/no_realloc.cc @@ -1,5 +1,6 @@ // { dg-do compile } // { dg-options "-O3 -g0" } +// { dg-require-normal-mode "" } // { dg-final { scan-assembler-not "_Znw" } } // GCC should be able to optimize away the paths involving reallocation. @@ -14,3 +15,8 @@ void fill_val(std::vector& vec, int i) { vec.assign(vec.size(), i); } + +void fill_range(std::vector& vec, const int* first) +{ + vec.assign(first, first + vec.size()); +} diff --git a/libstdc++-v3/testsuite/23_containers/vector/types/1.cc b/libstdc++-v3/testsuite/23_containers/vector/types/1.cc index 9be07d9fd5c..079e5af9556 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/types/1.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/types/1.cc @@ -18,7 +18,7 @@ // . // { dg-do compile } -// { dg-options "-Wno-unused-result -Wno-stringop-overread" } +// { dg-options "-Wno-unused-result" } #include #include