From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x635.google.com (mail-ej1-x635.google.com [IPv6:2a00:1450:4864:20::635]) by sourceware.org (Postfix) with ESMTPS id 9BB623858403; Fri, 2 Feb 2024 16:52:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9BB623858403 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 9BB623858403 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::635 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706892772; cv=none; b=ioTr6uxWtLI0aY67x+kfLP9a/QvuwueTsOZFaeDooVLHoXOc2ZNUPsHRDM+N2GKFxuRaRAmwkeNXE6tXW0I8aENxnoQzvYadooArSIsgZhBPN0qOIo0ZgDo7z1LM3ZflGaKJBkRm/nSPbs3hz7kaI3M7caDEqMY1TZdSjwD7kis= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706892772; c=relaxed/simple; bh=g4YQwWWtU71a3ApOw2qFmaT3RBv8kaI8TUvB8gKpvrQ=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=Hzi8qO9oJknuZl31+DIYmOWqNyMRbsqTbyfDzreo6b2A4F1z+ZXgn7uzMxFH1xq0uY7Wbk/h2zhJARJq4pxQ386NNuCnF9QrpY8VdsKCXOkq4hBVJobaDaIssaY56tPdt6FHTy4kVOxjTRIkknRBNVdJC5t3tvZR+0e4qQgPPAk= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-ej1-x635.google.com with SMTP id a640c23a62f3a-a3122b70439so318395266b.3; Fri, 02 Feb 2024 08:52:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1706892765; x=1707497565; darn=gcc.gnu.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=g4gjo/GFq/9V1SaKlZcbNjYGrAhbs6LlGMo+PruJp98=; b=ds8xmibsWjz7pMpXtAdVcjLlLHPkqq9DBjTeFgs5OGecQK6yVezgPcBLelrHmPnoGp 3l8e/kJ7O3yb84vUfydnhdCcvBmOfYobhcgs0mV9LdrG1yk052Orq5RkiAxnyyoFQUP7 vu5hZnLbJfjywqmZYD0HRSJApUHIFLKs5ioGJ4vEVypErsLYNdkcJ/8iyIv4/MD74s5H 3syySeQOBXhH6ctlzVRwop/AqdUZ5ydqIt55hAOi/BHQgaC3sp3UyHw8miNwuGCUg+yN 6ei1PnHxzTKT2UK5/JZYLaKwCtUt4eiUVHCzqMnjX+DeBNMad4+jrdDWNrzvYRNA6jKr shww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706892765; x=1707497565; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=g4gjo/GFq/9V1SaKlZcbNjYGrAhbs6LlGMo+PruJp98=; b=Dv61SrRaftz9f4bWKMezh0Jf7KPrWlusho5o9c2JBa23LPR5V3OCZ0Ip0eriZcb0kc 2doARTOW4edcKOdPiGGFXZKZiueC8uRxHFYglOUf+88n6Jq2IX18dWsOFK4WF88J4ljA ifgQmxej1jzk+BDMEHzYED3ApwqBMEK0holnylTZSuEsN/BGjNTbUqpOabJ3TY5LySiv WtZectcppEIG6MbPfBmIv3qTW1QZz1UGD+FD1m4ysRPOkDXKHGthnfswOyw1uPDH0HOY ZU1ilkLfZgenVDW8kWWJQbOzs8vJTUcK4tKMyNkP6BMN/9Xs3auQV8hwjEINCl0mc/rq 0eGw== X-Gm-Message-State: AOJu0YyJ2z9nwrJngqjrlFCZYuL2PQxuIXRBN1kizC1qgIluWcmR/kg9 S80c9ent4BG0Q+cQQp2all0fQCyZpUFWlJ4bwCr8foAFONaBHtUt8zjS/4vKco3YeAj+XS74Vri aLwKO9gOyWfIj+K6NVPKizWznytA= X-Google-Smtp-Source: AGHT+IF8tjhYjCWOopMzVZBWTxQ4w4jgkVmUcLGbdO5ynqXbqAPSOTVI+a5Sm3kA91pmLsgnS7+cs5suBAN44DouFCU= X-Received: by 2002:a17:906:3296:b0:a36:f27b:f0e with SMTP id 22-20020a170906329600b00a36f27b0f0emr1817652ejw.7.1706892764942; Fri, 02 Feb 2024 08:52:44 -0800 (PST) MIME-Version: 1.0 References: <9f2bdd3a973d98199bb5b322baa575ab2fba8a58.camel@xry111.site> In-Reply-To: From: xndcn Date: Sat, 3 Feb 2024 00:52:33 +0800 Message-ID: Subject: Re: [PATCH] libstdc++: atomic: Add missing clear_padding in __atomic_float constructor To: Jonathan Wakely Cc: Jakub Jelinek , Xi Ruoyao , "H.J. Lu" , GCC Patches , libstdc++@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,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 List-Id: Thank you for your careful review! > But we don't need a new one if it's going to be used in exactly one test and if the new option does the same thing for all targets that run the test. Got it, thanks. Now add option "-latomic" directly, but it still rely on the trick "[atomic_link_flags [get_multilibs]]" > No, because the patch is supposed to prevent the infinite loop, and so there's no need to stop it looping after 10s. It won't loop at all. Thanks, deleted. > We only need to clear padding for long double, not float and double, right? Yes, actually there is a check "if constexpr (__atomic_impl::__maybe_has_padding<_Fp>())". But "__atomic_impl::__clear_padding(_M_fp); " is indeed simply, so fixed here. > Why can't we run this on all targets? Got it, now target option deleted. > There's no reason to use __builtin_memset here, just include and use std::memcpy. Thanks, fixed. > It definitely does have padding, just say "long double has padding bits on x86" Thanks, fixed. So here comes the latest patch: --- libstdc++-v3/ChangeLog: * include/bits/atomic_base.h: add __builtin_clear_padding in __atomic_float constructor. * testsuite/29_atomics/atomic_float/compare_exchange_padding.cc: New test. Signed-off-by: xndcn --- libstdc++-v3/include/bits/atomic_base.h | 2 +- .../atomic_float/compare_exchange_padding.cc | 53 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc diff --git a/libstdc++-v3/include/bits/atomic_base.h b/libstdc++-v3/include/bits/atomic_base.h index f4ce0fa53..cdd6f07da 100644 --- a/libstdc++-v3/include/bits/atomic_base.h +++ b/libstdc++-v3/include/bits/atomic_base.h @@ -1283,7 +1283,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr __atomic_float(_Fp __t) : _M_fp(__t) - { } + { __atomic_impl::__clear_padding(_M_fp); } __atomic_float(const __atomic_float&) = delete; __atomic_float& operator=(const __atomic_float&) = delete; diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc new file mode 100644 index 000000000..eeace251c --- /dev/null +++ b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc @@ -0,0 +1,53 @@ +// { dg-do run { target c++20 } } +// { dg-options "-O0" } +// { dg-additional-options "[atomic_link_flags [get_multilibs]] -latomic" } + +#include +#include +#include +#include + +template +void __attribute__((noinline,noipa)) +fill_padding(T& f) +{ + T mask; + std::memset(&mask, 0xff, sizeof(T)); + __builtin_clear_padding(&mask); + unsigned char* ptr_f = (unsigned char*)&f; + unsigned char* ptr_mask = (unsigned char*)&mask; + for (int i = 0; i < sizeof(T); i++) + { + if (ptr_mask[i] == 0x00) + { + ptr_f[i] = 0xff; + } + } +} + +void +test01() +{ + // test for long double with padding (float80) + if constexpr (std::numeric_limits::digits == 64) + { + long double f = 0.5f; // long double has padding bits on x86 + fill_padding(f); + std::atomic as{ f }; // padding cleared on constructor + long double t = 1.5; + + as.fetch_add(t); + long double s = f + t; + t = as.load(); + VERIFY(s == t); // padding ignored on float comparing + fill_padding(s); + VERIFY(as.compare_exchange_weak(s, f)); // padding cleared on cmpexchg + fill_padding(f); + VERIFY(as.compare_exchange_strong(f, t)); // padding cleared on cmpexchg + } +} + +int main() +{ + test01(); +} -- 2.25.1