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.129.124]) by sourceware.org (Postfix) with ESMTPS id B8F4D3858CD1 for ; Tue, 9 Apr 2024 03:17:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B8F4D3858CD1 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 B8F4D3858CD1 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712632653; cv=none; b=Ek3Fztg/gqLSYu6etGw2mOZlXp8mWqqETwkCMImihFyKOhnVkWVuDAr9+r/SzCmIpRS6MQ4MErjOW3y0v52WP/1WQjcxu5dbzTNZimTiemLWQ76eiPI3dBzNGlOEs9GyFhLzWmsc2Fmgh8FGcZXK6wPSSq5T7yQysLm96K10nBM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712632653; c=relaxed/simple; bh=1O5+BagHKpyX2eGZGRN0YWkOPqm5T343FwdJhF4EZaA=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=UOpJtAZKO3TxCFzYBAoJKoEk0ftWjTddt9Xi8D5Q+OV74w5ccKHsNdIRYHIdDa/MqVj13fXBrpoz9tzq9V4AS9Xuc3mY0jLdMwiNI9LnY3GRwX7FkFW1KG9ysnN0/zn7n3XRRltlNHBFlh5kpxhgEXi7p4OrJKUKPUB8RaxUbQ4= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1712632651; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=lu8wvdPK3mvQIHJB66AYrWQe3B/iNKZeQjCuvEgc3AI=; b=QrfuZnxawOsqyUJrIXu6CLEELVoGEOypyykIwDCSAWHqgz1tPZE+v+stf7Xv9sChyuDXck GUs6UTbUAeZS74660ciwyJoyxBWCoEVKV7a/fkc0O8NHYrN0Q8xcJBaccDDIp9fbh/KT6l ZToe9zCVIWBdinISCwCTMF+sa/V1yuI= Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-377-dA16CcA7M4qfUCAvTt4aEQ-1; Mon, 08 Apr 2024 23:17:30 -0400 X-MC-Unique: dA16CcA7M4qfUCAvTt4aEQ-1 Received: by mail-qk1-f199.google.com with SMTP id af79cd13be357-7817253831cso677308785a.0 for ; Mon, 08 Apr 2024 20:17:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712632649; x=1713237449; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=lu8wvdPK3mvQIHJB66AYrWQe3B/iNKZeQjCuvEgc3AI=; b=b+6aFqjYI7MHlw39ja5Tm/lLt/9v7AUFHXs+56iwvgfUQAI/lZE2ljgZzPEkr8UJh7 z5ln1xwhtm5iuxk/HVRXxZpCieaNZ7Hbz11xXTqjRp8/TpMx14SRETzn1sxGYjNychN1 JGR8ADeSHVMhxPspVDhGx4Pccypc0JURKV+0/8B0yBrk5m31L3N22AgwNiqiXKVCbGmQ QWk/2viLcCtzUq1Q6S6kup73CDuH/E84eMS7s4EKmjQC2auFMm2F1T6L9LxHPD22xOhW fldB1bWo+V90dEDCgA7aMRkGbMBNNbwC88KjXALzJqRTaZHCfyBQ8fe+Pnrk2eU7wnux p9Eg== X-Gm-Message-State: AOJu0YxsgOO3WknsDeZpB0nOw/IjJzJ/9+QiTD8SHulppAlaT8LwqDHp hs3xUwVJwrpf3A6jiORVvLU80pvei+ubz2IEf9pH4Ox6CjSccsoSs6L0ahkbO7nNf1pYSXnpYXL Qn7+qn9VqfD8NCtkaoL6fybXDj8To1Mu6ajckbeisqccamxSXkOgwBaby+nBkNMw= X-Received: by 2002:a05:620a:a98:b0:78d:671c:ecd with SMTP id v24-20020a05620a0a9800b0078d671c0ecdmr4103236qkg.45.1712632649101; Mon, 08 Apr 2024 20:17:29 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHvH2fKojR1dNTimY+D2LSt/it7wFq9YOFKs4v2sQKkTArswycOSutDsm2Qg+WYzEmlbW7W8A== X-Received: by 2002:a05:620a:a98:b0:78d:671c:ecd with SMTP id v24-20020a05620a0a9800b0078d671c0ecdmr4103225qkg.45.1712632648778; Mon, 08 Apr 2024 20:17:28 -0700 (PDT) Received: from [192.168.1.130] (130-44-146-16.s12558.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.146.16]) by smtp.gmail.com with ESMTPSA id c6-20020a05620a0ce600b0078bcffed4acsm3756453qkj.19.2024.04.08.20.17.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 08 Apr 2024 20:17:28 -0700 (PDT) Message-ID: <40220469-1dfe-4d3f-9a3d-686a074f80b9@redhat.com> Date: Mon, 8 Apr 2024 23:17:27 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] c++: Keep DECL_SAVED_TREE of destructor instantiations in modules [PR104040] To: Nathaniel Shead Cc: gcc-patches@gcc.gnu.org, Nathan Sidwell References: <660633b1.170a0220.717f3.817f@mx.google.com> <660ca996.170a0220.60d95.0377@mx.google.com> <6ea38b8a-4146-4c02-b28a-80f1de178e01@redhat.com> <660e8ec1.170a0220.2a0c4.3501@mx.google.com> From: Jason Merrill In-Reply-To: <660e8ec1.170a0220.2a0c4.3501@mx.google.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,RCVD_IN_SORBS_WEB,SPF_HELO_NONE,SPF_NONE,TXREP 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 4/4/24 07:27, Nathaniel Shead wrote: > On Wed, Apr 03, 2024 at 11:18:01AM -0400, Jason Merrill wrote: >> On 4/2/24 20:57, Nathaniel Shead wrote: >>> On Tue, Apr 02, 2024 at 01:18:17PM -0400, Jason Merrill wrote: >>>> On 3/28/24 23:21, Nathaniel Shead wrote: >>>>> - && !(modules_p () && DECL_DECLARED_INLINE_P (fn))) >>>>> + && !(modules_p () >>>>> + && (DECL_DECLARED_INLINE_P (fn) >>>>> + || DECL_TEMPLATE_INSTANTIATION (fn)))) >>>> >>>> How about using vague_linkage_p? >>>> >>> >>> Right, of course. How about this? >>> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? >>> >>> -- >8 -- >>> >>> A template instantiation still needs to have its DECL_SAVED_TREE so that >>> its definition is emitted into the CMI. This way it can be emitted in >>> the object file of any importers that use it, in case it doesn't end up >>> getting emitted in this TU. >>> >>> PR c++/104040 >>> >>> gcc/cp/ChangeLog: >>> >>> * semantics.cc (expand_or_defer_fn_1): Keep DECL_SAVED_TREE for >>> all vague linkage functions. >>> >>> gcc/testsuite/ChangeLog: >>> >>> * g++.dg/modules/pr104040_a.C: New test. >>> * g++.dg/modules/pr104040_b.C: New test. >>> >>> Signed-off-by: Nathaniel Shead >>> Reviewed-by: Jason Merrill >>> --- >>> gcc/cp/semantics.cc | 5 +++-- >>> gcc/testsuite/g++.dg/modules/pr104040_a.C | 14 ++++++++++++++ >>> gcc/testsuite/g++.dg/modules/pr104040_b.C | 8 ++++++++ >>> 3 files changed, 25 insertions(+), 2 deletions(-) >>> create mode 100644 gcc/testsuite/g++.dg/modules/pr104040_a.C >>> create mode 100644 gcc/testsuite/g++.dg/modules/pr104040_b.C >>> >>> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc >>> index adb1ba48d29..03800a20b26 100644 >>> --- a/gcc/cp/semantics.cc >>> +++ b/gcc/cp/semantics.cc >>> @@ -5033,9 +5033,10 @@ expand_or_defer_fn_1 (tree fn) >>> /* We don't want to process FN again, so pretend we've written >>> it out, even though we haven't. */ >>> TREE_ASM_WRITTEN (fn) = 1; >>> - /* If this is a constexpr function, keep DECL_SAVED_TREE. */ >>> + /* If this is a constexpr function, or the body might need to be >>> + exported from a module CMI, keep DECL_SAVED_TREE. */ >>> if (!DECL_DECLARED_CONSTEXPR_P (fn) >>> - && !(modules_p () && DECL_DECLARED_INLINE_P (fn))) >>> + && !(modules_p () && vague_linkage_p (fn))) >> >> Also, how about module_maybe_has_cmi_p? OK with that change. > > Using 'module_maybe_has_cmi_p' doesn't seem to work. This is for two > reasons, one of them fixable and one of them not (easily): > > - It seems that header modules don't count for 'module_maybe_has_cmi_p'; > I didn't notice this initially, and maybe they should for the > no-linkage decls too? I think so; they could similarly be referred to by an importer. > But even accounting for this, > > - For some reason only clearing it if the module might have a CMI causes > crashes in importers for some testcases. I'm not 100% sure why yet, > but I suspect it might be some duplicate-decls thing where the type > inconsistently has DECL_SAVED_TREE applied, since this is also called > on streamed-in declarations. Clearing if the module might have a CMI sounds backwards, I'd expect that to be the case where we want to leave it alone. Is that the problem, or just a typo? > Out of interest, what was the reason that it was cleared at all in the > first place? I wasn't able to find anything with git blame; is it just > for performance reasons in avoiding excess lowering later? That change goes back to the LTO merge, I believe it was to reduce unnecessary LTO streaming. But now that I think about it some more, I don't see why handling modules specially here is necessary at all; the point of this code is that after we build the destructor clones, the DECL_SAVED_TREE of the cloned function is no longer useful. Why would modules care about the maybe-in-charge function? Jason