From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x62a.google.com (mail-ej1-x62a.google.com [IPv6:2a00:1450:4864:20::62a]) by sourceware.org (Postfix) with ESMTPS id 0EA603858C78; Wed, 31 Jan 2024 17:19:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0EA603858C78 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 0EA603858C78 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::62a ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706721601; cv=none; b=iDVg+NQE9ThC2FYsN/K7bVPU9rWkuXq4m1nghNp/Bfi9kZBI8sRvGmedIRf9fdcPfJQ/lD4Znteytya24GkzVJgnHdv0dGeUxLwRIWG+brWd0jpMREZ27ObF8vUzeAJ9arphdhY8Uu+N05b7u0rvxuAyM9uFsqt9bP/bxxmNE0s= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706721601; c=relaxed/simple; bh=1gIfmveHCkdh9QYk0KeNdSCCWobH5NrDZHpWmzIpN9M=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=Exy1M3iRl5YRPjLQ4BVh+M1B6A92ueSMWoUlMkS5mV5CaTAKtMx42Qreg3W7xFyqe2honLv4WoC6BxppGa9lkEeN3ymzoZ4DwRW9IA6c1BcCqhyklINNbEGlsDM0MMlkxxuFVaYlRvV5K6DAZGvuE4ed2i+CvXs3/Aodr8nYFDc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-ej1-x62a.google.com with SMTP id a640c23a62f3a-a26fa294e56so708804966b.0; Wed, 31 Jan 2024 09:19:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1706721596; x=1707326396; 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=pwztcGF8NXps9MHS7qjA3RSHnnRf2Fmk9fBGewuE6uU=; b=PHf/ax0HTWK/u+Xd74SBgY5reT1qLZTw7qlbo1d/EEiP816cAoJc62Jhuybv7vpo2s EZIvt1mvtZKtWXwYzbhHP3xXK4+KYoOhkau1DWYk+WEzSlJ2c8RD2hgnLWW0BU8skPIq 7NJCRAt18wZ2VwaazeqgYnQEaMLTqptk8vvPxy834q7qundwuCQ8uf4cPcEigwbjqRzy VTJbFxaZLjN3Mml3amB/Y3DBMR4OvHXHHOPb+pXNSO15FA4E+T9GwGzrqYkjJM3dQTTL XnfBHs1uygMza1g2too2C/wLdZIWGJ7PiFIX8x0AW0VmtS3poxhCRX9eCDlzKn8qScqI ZapA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706721596; x=1707326396; 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=pwztcGF8NXps9MHS7qjA3RSHnnRf2Fmk9fBGewuE6uU=; b=JTI4cdU6Jyx9poghbsrz+QcXim2L6cmMgql4NIuV/gUN86JGFeSwBZ8Ep0u6EbqXBC KbXd1NkZF7DCr9GnC42EUqZoqE8IiIbx2aefldYEOOfwwW4Z7wadB3OPpAYjsa4+khP3 jh/6uHFBjrQ/U/8Y6uWGhJIOoiHgUP48Aa1gmWFERZJ4QEXqoEoDoCfegCnvO/Wen+il /RUYIRiX0KNKG8n00Eds65cy5l8dly+R1lQJVom0qkwUmKuxOwk9SiLKL0VyOUlZRmZe 30fE85HRhdy0Z1PjoUN54cOTlKZjFklQq7jIQGEb6ogoiLIjTD+pa2VpJf5vUq8BA3gd ACiA== X-Gm-Message-State: AOJu0YztPxk5UcApkE1HWzqleCO9canmjy0rqfj13zdFIcaOipezFF50 mEllf0uTiyMCSoDCWkFK4Le79vbA4/VlHef/2g4VUBRxaeQkIA5h1fZMlxI31EUKeBjVhuooXxv r0RrxMJtf5As0FKbAGL+Urj9QOrI= X-Google-Smtp-Source: AGHT+IHxdE4QeZxax8IT9Hdv4KisOsYYBQNQIMNIKhBb7drJ3z/hhjrjTrf0exAFcA+BTgTKOc+iunEESMLvdgeGqu8= X-Received: by 2002:a17:906:5f96:b0:a35:b7e6:1c44 with SMTP id a22-20020a1709065f9600b00a35b7e61c44mr1650092eju.75.1706721596437; Wed, 31 Jan 2024 09:19:56 -0800 (PST) MIME-Version: 1.0 References: <9f2bdd3a973d98199bb5b322baa575ab2fba8a58.camel@xry111.site> In-Reply-To: From: xndcn Date: Thu, 1 Feb 2024 01:19:41 +0800 Message-ID: Subject: Re: [PATCH] libstdc++: atomic: Add missing clear_padding in __atomic_float constructor To: Jakub Jelinek Cc: Jonathan Wakely , Xi Ruoyao , "H.J. Lu" , GCC Patches , libstdc++@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.0 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: Thanks! > Why not just use -latomic unconditionally here? testsuites of libstdc++ seems to have some tricks to find and link libatomic.a by using "dg-add-options libatomic", which do nothing for x86 target. In previous email, we decide not to pollute the current option, so we add a new libatomic_16b option here. > Why dg-timeout? without the patch, "as.fetch_add(t);" will result in an infinite loop, so I add timeout to help it escape. Should I keep it or not? > Also, the target selectors above look wrong. Thanks, fixed with checking "std::numeric_limits::digits == 64". > This code is not compiled for C++11 and C++14, so this should just use > constexpr not _GLIBCXX17_CONSTEXPR. Thanks, fixed with "constexpr". So here is the fixed patch, please review it again, thanks: --- libstdc++-v3/ChangeLog: * include/bits/atomic_base.h: add __builtin_clear_padding in __atomic_float constructor. * testsuite/lib/dg-options.exp: add new add-options for libatomic_16b. * testsuite/29_atomics/atomic_float/compare_exchange_padding.cc: New test. Signed-off-by: xndcn --- libstdc++-v3/include/bits/atomic_base.h | 7 ++- .../atomic_float/compare_exchange_padding.cc | 54 +++++++++++++++++++ libstdc++-v3/testsuite/lib/dg-options.exp | 22 ++++++++ 3 files changed, 82 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..90e3f0e4b 100644 --- a/libstdc++-v3/include/bits/atomic_base.h +++ b/libstdc++-v3/include/bits/atomic_base.h @@ -1283,7 +1283,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr __atomic_float(_Fp __t) : _M_fp(__t) - { } + { +#if __has_builtin(__builtin_clear_padding) + if constexpr (__atomic_impl::__maybe_has_padding<_Fp>()) + __builtin_clear_padding(std::__addressof(_M_fp)); +#endif + } __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..e6e17e4b5 --- /dev/null +++ b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc @@ -0,0 +1,54 @@ +// { dg-do run { target c++20 } } +// { dg-options "-O0" } +// { dg-timeout 10 } +// { dg-do run { target { i?86-*-* x86_64-*-* } } } +// { dg-add-options libatomic_16b } + +#include +#include +#include + +template +void __attribute__((noinline,noipa)) +fill_padding(T& f) +{ + T mask; + __builtin_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 may contains padding 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(); +} diff --git a/libstdc++-v3/testsuite/lib/dg-options.exp b/libstdc++-v3/testsuite/lib/dg-options.exp index 850442b6b..75b27a136 100644 --- a/libstdc++-v3/testsuite/lib/dg-options.exp +++ b/libstdc++-v3/testsuite/lib/dg-options.exp @@ -356,6 +356,28 @@ proc add_options_for_libatomic { flags } { return $flags } +# Add option to link to libatomic, if required for atomics on 16-byte (128-bit) +proc add_options_for_libatomic_16b { flags } { + if { ([istarget i?86-*-*] || [istarget x86_64-*-*]) + } { + global TOOL_OPTIONS + + set link_flags "" + if ![is_remote host] { + if [info exists TOOL_OPTIONS] { + set link_flags "[atomic_link_flags [get_multilibs ${TOOL_OPTIONS}]]" + } else { + set link_flags "[atomic_link_flags [get_multilibs]]" + } + } + + append link_flags " -latomic " + + return "$flags $link_flags" + } + return $flags +} + # Add options to enable use of deprecated features. proc add_options_for_using-deprecated { flags } { return "$flags -U_GLIBCXX_USE_DEPRECATED -D_GLIBCXX_USE_DEPRECATED=1" -- 2.25.1