From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 53CA73858D20 for ; Tue, 20 Feb 2024 00:51:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 53CA73858D20 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 53CA73858D20 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=1708390315; cv=none; b=I+FOPFHN1YHnkMY3SL0jp6bpVz1GekVPqjNpW6/p9+P+3HB2yTLYYARQYVPoM7N28aCuxlC3G/QfzQ5jPXqoayc+ay6yBuEEt3SVIGlW+doKSh3OJO9UV3ISJWHlLHCRTH14nROl2ROfap1eAOBlhznWUNwWLpWFEMdRCu0BcAM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708390315; c=relaxed/simple; bh=jwUE49Vu0iOL1w+/lwQhM+rOfcH7W2oVG7ifuzRvhG8=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=Fw5JOW97/a19a9rZYSNPS16gYwZh5KX4roJ0iqaydsUHxQH43VcsePWAa/CmpvBYLoagGJMf2Cx/2N2DpHTXs4IQTSxVF9m5VeSQ0yvjgdgGU8fPADaqVrT2b0bBgGVNquMklqci94+cKmm9gs3BXyj0Si5vKiA3YVvpU1KAWRI= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1708390311; 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=mX1JGn7lr8ej3ttnMKCw1uVd/dRbRb0sLeswkBQFSUY=; b=MV53wWtCDsuO5SrOgcFhh5/6bVahYluAXfv49VcrrvidKPJ1v/InFIsq4mz/NmZZfbfeBv OIXpwoKdye3aunJTgC+4YamV7Y5BxB+gLdXH0MCRIQyX/d1rYPS1Pc0OPcskBbQA6JgMOg /CichftomfmLJWZwBnOoBzaAipsFYFk= 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-208-qvKIyTSvOj-KXxMcYiJMZQ-1; Mon, 19 Feb 2024 19:51:47 -0500 X-MC-Unique: qvKIyTSvOj-KXxMcYiJMZQ-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (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 7BAF21022EE2; Tue, 20 Feb 2024 00:51:47 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.8]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 24E771C060B2; Tue, 20 Feb 2024 00:51:47 +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 41K0pi4F430165 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Tue, 20 Feb 2024 01:51:44 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 41K0phr3430164; Tue, 20 Feb 2024 01:51:43 +0100 Date: Tue, 20 Feb 2024 01:51:43 +0100 From: Jakub Jelinek To: Jason Merrill , Richard Biener Cc: gcc-patches@gcc.gnu.org, Jonathan Wakely Subject: Re: [PATCH] c-family, c++: Fix up handling of types which may have padding in __atomic_{compare_}exchange Message-ID: Reply-To: Jakub Jelinek References: <5988812a-3f21-4ef4-9205-fe267c356d58@redhat.com> MIME-Version: 1.0 In-Reply-To: <5988812a-3f21-4ef4-9205-fe267c356d58@redhat.com> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii 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 List-Id: On Tue, Feb 20, 2024 at 12:12:11AM +0000, Jason Merrill wrote: > On 2/19/24 02:55, Jakub Jelinek wrote: > > 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); > > Hmm, I find that surprising; I thought of VIEW_CONVERT_EXPR as working on > lvalues, not rvalues, based on the documentation describing it as roughly > *(type2 *)&X. It works on both, if it is on the lhs, it obviously needs to be on an lvalue and is something like VIEW_CONVERT_EXPR (mem) = something; and if it is on rhs, it can be on an rvalue, just reinterpret bits of something as something else, so more like ((union { typeof (val) x; I_type y; }) (val)).y > Now I see that gimplify_expr does what you describe, and I wonder what the > advantage of that is. That seems to go back to richi's r206420 for PR59471. > r270579 for PR limited it to cases where the caller wants an rvalue; perhaps > it should also be avoided when the operand is an INDIRECT_REF? Strangely we don't even try to optimize it, even at -O2 that _3 = *p2_2(D); _4 = VIEW_CONVERT_EXPR (_3); stays around until optimized. I believe VIEW_CONVERT_EXPR is valid around a memory reference, so it could be either _4 = VIEW_CONVERT_EXPR (*p2_2(D)); or the MEM_REF with aliasing info from original pointer and type from VCE. For optimizations, guess it is a matter of writing some match.pd rule, but we can't rely on it for the atomics. Doing it in the gimplifier is possible, but not sure how to do that easily, given that the VIEW_CONVERT_EXPR argument can be either lvalue or rvalue and we need to gimplify it first. The first part is exactly what forces it into a separate SSA_NAME for the load vs. VIEW_CONVERT_EXPR around it: case VIEW_CONVERT_EXPR: if ((fallback & fb_rvalue) && is_gimple_reg_type (TREE_TYPE (*expr_p)) && is_gimple_reg_type (TREE_TYPE (TREE_OPERAND (*expr_p, 0)))) { ret = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p, is_gimple_val, fb_rvalue); recalculate_side_effects (*expr_p); break; } /* Fallthru. */ case ARRAY_REF: case ARRAY_RANGE_REF: case REALPART_EXPR: case IMAGPART_EXPR: case COMPONENT_REF: ret = gimplify_compound_lval (expr_p, pre_p, post_p, fallback ? fallback : fb_rvalue); break; but if we do the gimplify_compound_lval, we'd actually force it to be addressable (with possible errors if that isn't possible) etc. Having just a special-case of VIEW_CONVERT_EXPR of INDIRECT_REF and do gimplify_compound_lval in that case mikght be wrong I think if e.g. INDIRECT_REF operand is ADDR_EXPR, shouldn't we cancel *& in that case and still not force it into memory? The INDIRECT_REF: case is: { bool volatilep = TREE_THIS_VOLATILE (*expr_p); bool notrap = TREE_THIS_NOTRAP (*expr_p); tree saved_ptr_type = TREE_TYPE (TREE_OPERAND (*expr_p, 0)); *expr_p = fold_indirect_ref_loc (input_location, *expr_p); if (*expr_p != save_expr) { ret = GS_OK; break; } ret = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p, is_gimple_reg, fb_rvalue); if (ret == GS_ERROR) break; recalculate_side_effects (*expr_p); *expr_p = fold_build2_loc (input_location, MEM_REF, TREE_TYPE (*expr_p), TREE_OPERAND (*expr_p, 0), build_int_cst (saved_ptr_type, 0)); TREE_THIS_VOLATILE (*expr_p) = volatilep; TREE_THIS_NOTRAP (*expr_p) = notrap; ret = GS_OK; break; } so I think if we want to special case VIEW_CONVERT_EXPR on INDIRECT_REF, we should basically copy and adjust that to the start of the case VIEW_CONVERT_EXPR:. In particular, if fold_indirect_ref_loc did something and it isn't a different INDIRECT_REF or something addressable, do what it does now (i.e. possibly treat as lvalue), otherwise gimplify the INDIRECT_REF operand and build a MEM_REF, but with the type of the VIEW_CONVERT_EXPR but still saved_ptr_type of the INDIRECT_REF. Though, that all still feels like an optimization rather than guarantee which is what we need for the atomics. Jakub