From patchwork Mon Feb 19 07:55:01 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 202940 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:693c:2685:b0:108:e6aa:91d0 with SMTP id mn5csp1136909dyc; Sun, 18 Feb 2024 23:55:50 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCWUBFvjxFsmtAn31uCS5eZylZjd3OKIBM/I1ZjH8QidwnMvzINeCzWhxYfls30OIPcn/opar5Ce0cRsXVqyhx8gy5sLeA== X-Google-Smtp-Source: AGHT+IFyJ7QjQkzW88ujKb8pUdW+hVpjCp6xEFBfMhEmgwyTe0FbLTjwq26twlHM0TyM2UhxSM69 X-Received: by 2002:a05:620a:957:b0:787:245a:8115 with SMTP id w23-20020a05620a095700b00787245a8115mr12823225qkw.39.1708329350552; Sun, 18 Feb 2024 23:55:50 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1708329350; cv=pass; d=google.com; s=arc-20160816; b=zol1ZMGsi6jAZM9Ta69vVUOUBx0qvmX9PEve10oQ+DPBD0g8orrh01PNhZO86XzTvC suk7ZYs14EUsy0O5OKCvioHeZeb04SvFa58JknW/Vao0tsZlruWswLq9MgzMnl75Bcy0 7R0LR+hGPXDT0mUviDPVtiTppmAGkykepjWZY+ZKOMBz+/R9ILi+FVK9Zt4mgZrYk75P 0dMh78njIMRnjfH+fknJqD/wJ3k/2RevYjd8QozXPg+Sh+/TvyO1cWDfV273VP2QJw8z wEND4WWEtJVMitYzR+K9zzKsMZOEYZmRYusl8RnkKmKwhWKfVZbNClUGIXLC210v2OhQ 3APg== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:reply-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-disposition:in-reply-to :mime-version:references:message-id:subject:cc:to:from:date :dkim-signature:arc-filter:dmarc-filter:delivered-to; bh=n5+hQ4eEN2hHhUAjCN4TugbkIzu881j121WlFUAcW3Q=; fh=ZnRhuLcZE7P5g71He2CJmbCQxfFP+x15qvAW/SgLfEw=; b=QUE/I9pgODynZhGmlfnFNlgp0+UPGMEiL/R0r+mWTWMBMnKUgtxDjARZ9UHZcQbQgx kVeYUFjBeLB/1VKBsZT5EiyEDyGJQaOYFAruaVEH3sMD/La5LvTM0ewGTvfFZyl1w/Iz +xsn1fzj1SxJ+aXlqUVkApZXqq4txm9KufIXEY4vhJgcGBFpie/mUvoeznEiPajkyHpp XixZOmHRD6ookkoGs/nMZwkNz0F9SboV4LZFqxan/1OgJa+7G1pfk5xpucLMRVSUgWfa z54hZFbzmERt8OSjhZgXTDa4TlBKrNZBcKpH79Bl0n4eI0h6ask/t8yFTMpFg0F0Uyt1 D7sQ==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Jne0A4vK; arc=pass (i=1); spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from server2.sourceware.org (server2.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id bj24-20020a05620a191800b0078753d41a8bsi4850265qkb.742.2024.02.18.23.55.50 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 18 Feb 2024 23:55:50 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Jne0A4vK; arc=pass (i=1); spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 44CFD386186D for ; Mon, 19 Feb 2024 07:55:50 +0000 (GMT) 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 DA3313861871 for ; Mon, 19 Feb 2024 07:55:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DA3313861871 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org DA3313861871 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708329312; cv=none; b=nxSp278V1bIHiPWdS0Hl8W6ZuxNxix8NlsdqvRAd3eCIb1BoPa5xKaav/OGrw74bmEPNsF5z7BoIVpGAL6tmhHbdxPiPVrFsbzZpJlGlUbuJbQaeD3Erptr+NhlHYK6gWNAFhJJPTisOCrBHJrJbjxD+Uhma72DsUMcPxuLmuTk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708329312; c=relaxed/simple; bh=FtRpifvsdpXb0a+DNX5u5vVpwTvZ0hLx9cBeLX0TO6g=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=AmUtdB1naZ5boDWTdoMi3hh2q8XScgQcB+1qBusgp/xF4lm50fcuaF/VIo0VobZHRvEu7XiYoC51YTA8p1InCa8nvtPJECppcDBG2EXvyEmVGT+N7qoN9HV5yYUl26G0S8EeN3gQx+/3rIwGMNx4R+nX/v8lMqcXWyZcVHkP6nY= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1708329308; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references; bh=n5+hQ4eEN2hHhUAjCN4TugbkIzu881j121WlFUAcW3Q=; b=Jne0A4vKPEilsi912NBxxn25dBCX6UQ7E7iHG/b6IZKpiJ1c1zSIb+39oCtDzwGSmKKrMC GBe4yb+mLIQmxeJz9EviZDIGaL/+VlAFNWB4sWAwKPhq4E8cJ+cSUWoijjBBSpmlwZ7TgK pxtU1AGLCM3Fve4MHeZK7AtHLodRlfA= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-610-afMpaH5UN3SDD745dCFVLw-1; Mon, 19 Feb 2024 02:55:05 -0500 X-MC-Unique: afMpaH5UN3SDD745dCFVLw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 0F75B848524 for ; Mon, 19 Feb 2024 07:55:05 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.8]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C49F51121306; Mon, 19 Feb 2024 07:55:04 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 41J7t2fK1789662 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Mon, 19 Feb 2024 08:55:02 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 41J7t1hv1789661; Mon, 19 Feb 2024 08:55:01 +0100 Date: Mon, 19 Feb 2024 08:55:01 +0100 From: Jakub Jelinek To: Jason Merrill Cc: gcc-patches@gcc.gnu.org, Jonathan Wakely Subject: [PATCH] c-family, c++: Fix up handling of types which may have padding in __atomic_{compare_}exchange Message-ID: References: <9f2bdd3a973d98199bb5b322baa575ab2fba8a58.camel@xry111.site> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-3.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, 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.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Jakub Jelinek Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1791313156665199444 X-GMAIL-MSGID: 1791313156665199444 On Fri, Feb 16, 2024 at 01:51:54PM +0000, Jonathan Wakely wrote: > Ah, although __atomic_compare_exchange only takes pointers, the > compiler replaces that with a call to __atomic_compare_exchange_n > which takes the newval by value, which presumably uses an 80-bit FP > register and so the padding bits become indeterminate again. The problem is that __atomic_{,compare_}exchange lowering if it has a supported atomic 1/2/4/8/16 size emits code like: _3 = *p2; _4 = VIEW_CONVERT_EXPR (_3); so if long double or some small struct etc. has some carefully filled padding bits, those bits can be lost on the assignment. The library call for __atomic_{,compare_}exchange would actually work because it woiuld load the value from memory using integral type or memcpy. E.g. on void foo (long double *a, long double *b, long double *c) { __atomic_compare_exchange (a, b, c, false, __ATOMIC_RELAXED, __ATOMIC_RELAXED); } we end up with -O0 with: fldt (%rax) fstpt -48(%rbp) movq -48(%rbp), %rax movq -40(%rbp), %rdx i.e. load *c from memory into 387 register, store it back to uninitialized stack slot (the padding bits are now random in there) and then load a __uint128_t (pair of GPR regs). The problem is that we first load it using whatever type the pointer points to and then VIEW_CONVERT_EXPR that value: p2 = build_indirect_ref (loc, p2, RO_UNARY_STAR); p2 = build1 (VIEW_CONVERT_EXPR, I_type, p2); The following patch fixes that by creating a MEM_REF instead, with the I_type type, but with the original pointer type on the second argument for aliasing purposes, so we actually preserve the padding bits that way. I've done that for types which may have padding and also for non-integral/pointer types, because I fear even on floating point types like double or float which don't have padding bits the copying through floating point could misbehave in presence of sNaNs or unsupported bit combinations. With this patch instead of the above assembly we emit movq 8(%rax), %rdx movq (%rax), %rax I had to add support for MEM_REF in pt.cc, though with the assumption that it has been already originally created with non-dependent types/operands (which is the case here for the __atomic*exchange lowering). Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-02-19 Jakub Jelinek gcc/c-family/ * c-common.cc (resolve_overloaded_atomic_exchange): For non-integral/pointer types or types which may need padding instead of setting p1 to VIEW_CONVERT_EXPR (*p1), set it to MEM_REF with p1 and (typeof (p1)) 0 operands and I_type type. (resolve_overloaded_atomic_compare_exchange): Similarly for p2. gcc/cp/ * pt.cc (tsubst_expr): Handle MEM_REF. gcc/testsuite/ * g++.dg/ext/atomic-5.C: New test. Jakub --- gcc/c-family/c-common.cc.jj 2024-02-16 17:33:43.995739790 +0100 +++ gcc/c-family/c-common.cc 2024-02-17 11:11:34.029474214 +0100 @@ -7794,8 +7794,23 @@ resolve_overloaded_atomic_exchange (loca p0 = build1 (VIEW_CONVERT_EXPR, I_type_ptr, p0); (*params)[0] = p0; /* Convert new value to required type, and dereference it. */ - p1 = build_indirect_ref (loc, p1, RO_UNARY_STAR); - p1 = build1 (VIEW_CONVERT_EXPR, I_type, p1); + if ((!INTEGRAL_TYPE_P (TREE_TYPE (TREE_TYPE (p1))) + && !POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (p1)))) + || clear_padding_type_may_have_padding_p (TREE_TYPE (TREE_TYPE (p1)))) + { + /* If *p1 type can have padding or may involve floating point which + could e.g. be promoted to wider precision and demoted afterwards, + state of padding bits might not be preserved. */ + build_indirect_ref (loc, p1, RO_UNARY_STAR); + p1 = build2_loc (loc, MEM_REF, I_type, + build1 (VIEW_CONVERT_EXPR, I_type_ptr, p1), + build_zero_cst (TREE_TYPE (p1))); + } + else + { + p1 = build_indirect_ref (loc, p1, RO_UNARY_STAR); + p1 = build1 (VIEW_CONVERT_EXPR, I_type, p1); + } (*params)[1] = p1; /* Move memory model to the 3rd position, and end param list. */ @@ -7874,8 +7889,23 @@ resolve_overloaded_atomic_compare_exchan (*params)[1] = p1; /* Convert desired value to required type, and dereference it. */ - p2 = build_indirect_ref (loc, p2, RO_UNARY_STAR); - p2 = build1 (VIEW_CONVERT_EXPR, I_type, p2); + if ((!INTEGRAL_TYPE_P (TREE_TYPE (TREE_TYPE (p2))) + && !POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (p2)))) + || clear_padding_type_may_have_padding_p (TREE_TYPE (TREE_TYPE (p2)))) + { + /* If *p2 type can have padding or may involve floating point which + could e.g. be promoted to wider precision and demoted afterwards, + state of padding bits might not be preserved. */ + build_indirect_ref (loc, p2, RO_UNARY_STAR); + p2 = build2_loc (loc, MEM_REF, I_type, + build1 (VIEW_CONVERT_EXPR, I_type_ptr, p2), + build_zero_cst (TREE_TYPE (p2))); + } + else + { + p2 = build_indirect_ref (loc, p2, RO_UNARY_STAR); + p2 = build1 (VIEW_CONVERT_EXPR, I_type, p2); + } (*params)[2] = p2; /* The rest of the parameters are fine. NULL means no special return value --- gcc/cp/pt.cc.jj 2024-02-14 14:26:19.695811596 +0100 +++ gcc/cp/pt.cc 2024-02-17 11:05:47.988237152 +0100 @@ -20088,6 +20088,14 @@ tsubst_expr (tree t, tree args, tsubst_f RETURN (r); } + case MEM_REF: + { + tree op0 = RECUR (TREE_OPERAND (t, 0)); + tree op1 = RECUR (TREE_OPERAND (t, 0)); + tree new_type = tsubst (TREE_TYPE (t), args, complain, in_decl); + RETURN (build2_loc (EXPR_LOCATION (t), MEM_REF, new_type, op0, op1)); + } + case NOP_EXPR: { tree type = tsubst (TREE_TYPE (t), args, complain, in_decl); --- gcc/testsuite/g++.dg/ext/atomic-5.C.jj 2024-02-17 11:13:37.824770288 +0100 +++ gcc/testsuite/g++.dg/ext/atomic-5.C 2024-02-17 11:14:54.077720732 +0100 @@ -0,0 +1,42 @@ +// { dg-do compile { target c++14 } } + +template +void +foo (long double *ptr, long double *val, long double *ret) +{ + __atomic_exchange (ptr, val, ret, __ATOMIC_RELAXED); +} + +template +bool +bar (long double *ptr, long double *exp, long double *des) +{ + return __atomic_compare_exchange (ptr, exp, des, false, + __ATOMIC_RELAXED, __ATOMIC_RELAXED); +} + +bool +baz (long double *p, long double *q, long double *r) +{ + foo<0> (p, q, r); + foo<1> (p + 1, q + 1, r + 1); + return bar<0> (p + 2, q + 2, r + 2) || bar<1> (p + 3, q + 3, r + 3); +} + +constexpr int +qux (long double *ptr, long double *val, long double *ret) +{ + __atomic_exchange (ptr, val, ret, __ATOMIC_RELAXED); + return 0; +} + +constexpr bool +corge (long double *ptr, long double *exp, long double *des) +{ + return __atomic_compare_exchange (ptr, exp, des, false, + __ATOMIC_RELAXED, __ATOMIC_RELAXED); +} + +long double a[6]; +const int b = qux (a, a + 1, a + 2); +const bool c = corge (a + 3, a + 4, a + 5);