From patchwork Thu Dec 2 16:55:09 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 48423 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5ADD4385DC00 for ; Thu, 2 Dec 2021 17:14:54 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5ADD4385DC00 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1638465294; bh=1Airc2ofN52LQpGNB7RqHl8K7vWLieMiGVtrhL/tg8g=; h=References:In-Reply-To:Date:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=o7behzUrwYaTZXTTn6MaOv2BAoNVIz2vbcV0UR5R471110PSkGW2Il3O9YVWsrodp Oe+NDq0sIR7DfbSja9r11nJj97k2QuHkrsfFo9ChM79xqUUnEEDXHb4cgrEjDPOSow hQI5HYn5TSa6UcXvpwPVpoQREN66gvxovmHW4uuY= 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.133.124]) by sourceware.org (Postfix) with ESMTPS id 3F1A8385C40C for ; Thu, 2 Dec 2021 16:55:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3F1A8385C40C Received: from mail-yb1-f200.google.com (mail-yb1-f200.google.com [209.85.219.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-458-ppzJUjtrOF-JlXp0o-fE8A-1; Thu, 02 Dec 2021 11:55:21 -0500 X-MC-Unique: ppzJUjtrOF-JlXp0o-fE8A-1 Received: by mail-yb1-f200.google.com with SMTP id v7-20020a25ab87000000b005c2130838beso1029599ybi.0 for ; Thu, 02 Dec 2021 08:55:21 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=1Airc2ofN52LQpGNB7RqHl8K7vWLieMiGVtrhL/tg8g=; b=saJfpRhaEmghJVM6b4t5TWfR4vRsqv71PuvVXBlausHmzBxlHV68VVlspKiLlfeRft 03gaqDMkk1aChbmYZbMlRLVaQDI+cl37RhaVulTeKtF1ur9oof9yHlvkraprdopnGtDz 8hrzLBD2cihl+ZSRTTuuSJ+OI2p56AeiWr6ESD3cCGs87XHltJf3Bl8UPcPkppE1yJl5 fksfFIOBHKTwVAZL2xKUCYZ/1eR0fTv+2mjDRJ1jI+6oUJZMxZvjs8Y67+qCs5DTxh4j ySZxRe1NRE/PaqrIQChlQ5CSYHVo/P7TGrEgjtB5HpzwPISN1Wgk2PPuAi2UN7ozsuNL Xdqw== X-Gm-Message-State: AOAM530so8o5KJgW5F9RfKPmstBhVjinQ64HQCdq7wLcvS0KYefQu4ag cz59TM3ph404/+WLAhKAy91opZ7a51m2FQ/WEVeZ3mBwmYsH22TPDCQDGNfmYAxIY5iMIFTQ6XF mFCmGIg6DOu9cL2CdFQieSrT/vzwMh2JGqA== X-Received: by 2002:a25:f608:: with SMTP id t8mr16224606ybd.667.1638464121120; Thu, 02 Dec 2021 08:55:21 -0800 (PST) X-Google-Smtp-Source: ABdhPJxeVkEBXbt+0PMRJXThWkqFlw7XbXLhbxUxyGxWHePhPAIZut1ksCdy0tvw3OnRymI69atulKpolGj2W9bw1O4= X-Received: by 2002:a25:f608:: with SMTP id t8mr16224572ybd.667.1638464120867; Thu, 02 Dec 2021 08:55:20 -0800 (PST) MIME-Version: 1.0 References: <20211201150816.217497-1-jwakely@redhat.com> <87pmqg6vhw.fsf@oldenburg.str.redhat.com> In-Reply-To: Date: Thu, 2 Dec 2021 16:55:09 +0000 Message-ID: Subject: [committed] libstdc++: Restore unconditional atomic load in COW std::string To: Florian Weimer X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-13.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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 Cc: Jonathan Wakely via Libstdc++ , gcc Patches Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" On Wed, 1 Dec 2021 at 18:24, Jonathan Wakely wrote: > > On Wed, 1 Dec 2021 at 18:16, Florian Weimer wrote: > > > > * Jonathan Wakely via Libstdc: > > > > > diff --git a/libstdc++-v3/include/bits/cow_string.h b/libstdc++-v3/include/bits/cow_string.h > > > index ced395b80b8..4fae1d02981 100644 > > > --- a/libstdc++-v3/include/bits/cow_string.h > > > +++ b/libstdc++-v3/include/bits/cow_string.h > > > @@ -105,7 +105,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > > * destroy the empty-string _Rep object. > > > * > > > * All but the last paragraph is considered pretty conventional > > > - * for a C++ string implementation. > > > + * for a Copy-On-Write C++ string implementation. > > > */ > > > // 21.3 Template class basic_string > > > template > > > @@ -207,10 +207,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > > // so we need to use an atomic load. However, _M_is_leaked > > > // predicate does not change concurrently (i.e. the string is either > > > // leaked or not), so a relaxed load is enough. > > > - return __atomic_load_n(&this->_M_refcount, __ATOMIC_RELAXED) < 0; > > > -#else > > > - return this->_M_refcount < 0; > > > + if (!__gnu_cxx::__is_single_threaded()) > > > + return __atomic_load_n(&this->_M_refcount, __ATOMIC_RELAXED) < 0; > > > #endif > > > + return this->_M_refcount < 0; > > > } > > > > Relaxed MO loads of word-size values on all current architectures only > > have a compiler barrier, so I think the optimization makes things worse? > > Hmm, yes. > > > (I doubt the conditional lack of a compiler barrier leads to > > optimization improvements elsewhere.) > > Probably not. I'll revert the change to _M_is_leaked() and just keep > it for _M_is_shared(). > > Thanks for pointing that out. Reverted by the attached patch, tested powerpc64le-linux. commit b5a568683f71b4a8b1e4e45a43484398e9a66ff2 Author: Jonathan Wakely Date: Wed Dec 1 20:58:58 2021 libstdc++: Restore unconditional atomic load in COW std::string The relaxed load is already optimal, checking the __single_threaded global before doing a non-atomic load isn't an optimization. libstdc++-v3/ChangeLog: * include/bits/cow_string.h (basic_string::_M_is_leaked()): Revert change to check __is_single_threaded() before using atomic load. diff --git a/libstdc++-v3/include/bits/cow_string.h b/libstdc++-v3/include/bits/cow_string.h index d6ddf3489d1..389b39583e4 100644 --- a/libstdc++-v3/include/bits/cow_string.h +++ b/libstdc++-v3/include/bits/cow_string.h @@ -207,10 +207,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // so we need to use an atomic load. However, _M_is_leaked // predicate does not change concurrently (i.e. the string is either // leaked or not), so a relaxed load is enough. - if (!__gnu_cxx::__is_single_threaded()) - return __atomic_load_n(&this->_M_refcount, __ATOMIC_RELAXED) < 0; -#endif + return __atomic_load_n(&this->_M_refcount, __ATOMIC_RELAXED) < 0; +#else return this->_M_refcount < 0; +#endif } bool