From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by sourceware.org (Postfix) with ESMTPS id 8FC74386F436; Mon, 1 Mar 2021 19:44:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8FC74386F436 IronPort-SDR: uNG4C5jAU/V0xYUDXtL1+o2UtY20WSWY40TTJZy9JGre3JXSJr4vnYWYjhwSlk64JTBdxgPL7g SVL1s+YyZJwg== X-IronPort-AV: E=McAfee;i="6000,8403,9910"; a="166457303" X-IronPort-AV: E=Sophos;i="5.81,215,1610438400"; d="scan'208";a="166457303" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Mar 2021 11:44:33 -0800 IronPort-SDR: /FEZqrStbvn3VZW0FWs1W2DCWW0PYO+miamF9FoNomHDkfwrZvvveYhWXI7gfoeHS2vbjFsq/9 q4h4Ztdkvzow== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.81,215,1610438400"; d="scan'208";a="585611433" Received: from irsmsx605.ger.corp.intel.com ([163.33.146.138]) by orsmga005.jf.intel.com with ESMTP; 01 Mar 2021 11:44:32 -0800 Received: from tjmaciei-mobl1.localnet (10.255.230.217) by IRSMSX605.ger.corp.intel.com (163.33.146.138) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2106.2; Mon, 1 Mar 2021 19:44:30 +0000 From: Thiago Macieira To: Ville Voutilainen , libstdc++ CC: gcc-patches List Subject: Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state Date: Mon, 1 Mar 2021 11:44:27 -0800 Message-ID: <4029982.EvYjRiDrqD@tjmaciei-mobl1> Organization: Intel Corporation In-Reply-To: References: <1968544.UC5HiB4uFJ@tjmaciei-mobl1> <2595163.O2iKbAuXug@tjmaciei-mobl1> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Originating-IP: [10.255.230.217] X-ClientProxiedBy: orsmsx603.amr.corp.intel.com (10.22.229.16) To IRSMSX605.ger.corp.intel.com (163.33.146.138) X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Mar 2021 19:44:37 -0000 On Monday, 1 March 2021 10:12:35 PST Ville Voutilainen wrote: > I do have a question about the intent/concern here, regardless of what > your patch technically > does. The ABI break _is_ your concern, and the "heisenbugs" you were > worried about would > in fact be caused by the ABI break? So if you embed these things in > your QAtomicThing, the ABI > break may mess it up(*)? Is that a correct understanding of the concern > here? Let me clarify. I am stating that the current implementation is inefficient because Linux currently only implements 32-bit futexes. With that in mind, I am arguing we need to change the implementation in a way that will break ABI in a new release. The concern is that such a change, despite being in experimental code for which ABI stability has never been promised, is still troublesome. The problem there is that even people who have read the documentation and waited until 2024 to write code using the feature may still be affected. This isn't about Qt because I have no plans to use wait and notify in inline API. But you can see someone doing: #if __cplusplus >= 202002L && __has_include() # include #else # error "Please upgrade your compiler & standard library" #endif and using in their inline code. And as you say, if they then mix DSOs compiled with different versions of GCC, one of them might be the old, experimental and binary-incompatible code. Remember that before April 2024, the most recent Ubuntu LTS will be 22.04, which will be released before GCC 12, which means it will contain GCC 11. So, wholly aside from how we fix the inefficiencies, we must decide how to deal with the ABI break. We can: a) not have an ABI break in the first place, by having the above code not compile with GCC until we promise ABI compatibility b) cause a non-silent ABI break c) accept the silent ABI break I advocate for (a) and vehemently disagree with (c). Meanwhile, (b) can be something like (untested!): _GLIBCXX_BEGIN_NAMESPACE_VERSION namespace __detail { + struct __waiters; + inline namespace __experimental + { + // from atomic_wait.cc + __waiters &__get_waiter(const void *); + } using __platform_wait_t = int; constexpr auto __atomic_spin_count_1 = 16; @@ -187,11 +193,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static __waiters& _S_for(const void* __t) { - const unsigned char __mask = 0xf; - static __waiters __w[__mask + 1]; - - auto __key = _Hash_impl::hash(__t) & __mask; - return __w[__key]; + return __get_waiter(__t); } }; That way, any DSO compiled using GCC 11 will fail to load when using GCC 12's libstdc++. And probably put "std::__detail::__experimental" in the GLIBCXX_EXPERIMENTAL ELF version so it's even clearer that it's experimental, thus helping Linux distributions and other binary artefact distributors realise they've made a mistake. I still don't like (b) because it pushes the problem to the wrong people: the packagers. But it's far better than (c). -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel DPG Cloud Engineering