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 C4E1D3858D39 for ; Thu, 9 May 2024 18:20:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C4E1D3858D39 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 C4E1D3858D39 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=1715278809; cv=none; b=x53ypZ3Zm4hqgIjNm/XMna9XD61j32P9stAanFdqfoYk8VOZsUi0k87v3fQNZmzSVpuWN6zMejS0Oq2Fhuf8mWRBWpf7/Vljg/h3PonXg0SczA/datuip0ofBCgjDo7+sSwwBz+0V/6dC656t24TJMq0WlLqZOzZZZRLhcrboi0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715278809; c=relaxed/simple; bh=CyLqMuw92gf6t7NKZWakPF/U8cOv4IwxBqr9L3gAjEc=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=tkz37b04UmvQx3dCCvNz3Zaci4FU+T7GhdUap8jrr5LXZfKcsiN8TaoyiglsXKEE+VUxxZ4wwSM9fgMDFzzepkdag0LQ6o/s1XOQMgsKjarNRWNjrNnaTDKtmojL6pR04cb+OaWiS3kyXfTMAGkrIZM8Z0zbExWiMysQtHt8I/4= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1715278806; 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=CblNl1ihL59p/TlEcgcNKzV3fvcVZBjtm+eXIZJTnpI=; b=DvMwVYra37Ged0CHpbNT2UjUrty7s7qPwk2807j4rBxdxJS2TxBTMhIVuNFxppmFp+J2xS WB4B7xIN+4sA23v96B76WvWIUhU9tU19VqsrBZwVo0AGNzcx9vssaO+zfXlSgz+AjpD5Jd sR6jCX9UNy7xysRQoaTreZNK2tmZBng= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-153-o-FvPcEyO0qYoUnCqERBug-1; Thu, 09 May 2024 14:20:03 -0400 X-MC-Unique: o-FvPcEyO0qYoUnCqERBug-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (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 EF16D29AC025; Thu, 9 May 2024 18:20:02 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.45.224.64]) by smtp.corp.redhat.com (Postfix) with ESMTPS id AFEC0102966; Thu, 9 May 2024 18:20:02 +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 449IK1HO502876 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 9 May 2024 20:20:01 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 449IK0EP502875; Thu, 9 May 2024 20:20:00 +0200 Date: Thu, 9 May 2024 20:20:00 +0200 From: Jakub Jelinek To: Jason Merrill Cc: Jonathan Wakely , Jan Hubicka , gcc-patches@gcc.gnu.org, Richard Biener , Patrick Palka Subject: [PATCH] c++: Optimize in maybe_clone_body aliases even when not at_eof [PR113208] Message-ID: Reply-To: Jakub Jelinek References: <5fe9db40-4bff-4be4-a434-6509133f26e3@redhat.com> <5f6569bc-8d94-434c-b66d-137c8e7f5e64@redhat.com> <32bfcf8c-1b45-444c-8729-e560952fe44b@redhat.com> MIME-Version: 1.0 In-Reply-To: <32bfcf8c-1b45-444c-8729-e560952fe44b@redhat.com> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.5 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.9 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 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 Thu, Apr 25, 2024 at 11:30:48AM -0400, Jason Merrill wrote: > Hmm, maybe maybe_clone_body shouldn't clear DECL_SAVED_TREE for aliases, but > rather set it to some stub like void_node? > > Though with all these changes, it's probably better to go with your first > patch for GCC 14 and delay this approach to 15. Your v1 patch is OK for 14. Ok, here is an updated patch, which sets DECL_SAVED_TREE to void_node for the aliases together with reversion of the earlier committed patch. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-05-09 Jakub Jelinek Jason Merrill PR lto/113208 * cp-tree.h (maybe_optimize_cdtor): Remove. * decl2.cc (tentative_decl_linkage): Call maybe_make_one_only for implicit instantiations of maybe in charge ctors/dtors declared inline. (import_export_decl): Don't call maybe_optimize_cdtor. (c_parse_final_cleanups): Formatting fixes. * optimize.cc (can_alias_cdtor): Adjust condition, for HAVE_COMDAT_GROUP && DECL_ONE_ONLY && DECL_WEAK return true even if not DECL_INTERFACE_KNOWN. (maybe_clone_body): Don't clear DECL_SAVED_TREE, instead set it to void_node. (maybe_clone_body): Remove. * decl.cc (cxx_comdat_group): For DECL_CLONED_FUNCTION_P functions if SUPPORTS_ONE_ONLY return DECL_COMDAT_GROUP if already set. * g++.dg/abi/comdat3.C: New test. * g++.dg/abi/comdat4.C: New test. --- gcc/cp/cp-tree.h.jj 2024-05-09 10:30:54.775505524 +0200 +++ gcc/cp/cp-tree.h 2024-05-09 17:07:01.246206288 +0200 @@ -7451,7 +7451,6 @@ extern bool handle_module_option (unsign /* In optimize.cc */ extern tree clone_attrs (tree); extern bool maybe_clone_body (tree); -extern void maybe_optimize_cdtor (tree); /* In parser.cc */ extern tree cp_convert_range_for (tree, tree, tree, cp_decomp *, bool, --- gcc/cp/decl2.cc.jj 2024-05-02 09:31:17.753298180 +0200 +++ gcc/cp/decl2.cc 2024-05-09 17:11:11.676836268 +0200 @@ -3325,16 +3325,23 @@ tentative_decl_linkage (tree decl) linkage of all functions, and as that causes writes to the data mapped in from the PCH file, it's advantageous to mark the functions at this point. */ - if (DECL_DECLARED_INLINE_P (decl) - && (!DECL_IMPLICIT_INSTANTIATION (decl) - || DECL_DEFAULTED_FN (decl))) + if (DECL_DECLARED_INLINE_P (decl)) { - /* This function must have external linkage, as - otherwise DECL_INTERFACE_KNOWN would have been - set. */ - gcc_assert (TREE_PUBLIC (decl)); - comdat_linkage (decl); - DECL_INTERFACE_KNOWN (decl) = 1; + if (!DECL_IMPLICIT_INSTANTIATION (decl) + || DECL_DEFAULTED_FN (decl)) + { + /* This function must have external linkage, as + otherwise DECL_INTERFACE_KNOWN would have been + set. */ + gcc_assert (TREE_PUBLIC (decl)); + comdat_linkage (decl); + DECL_INTERFACE_KNOWN (decl) = 1; + } + else if (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)) + /* For implicit instantiations of cdtors try to make + it comdat, so that maybe_clone_body can use aliases. + See PR113208. */ + maybe_make_one_only (decl); } } else if (VAR_P (decl)) @@ -3604,9 +3611,6 @@ import_export_decl (tree decl) } DECL_INTERFACE_KNOWN (decl) = 1; - - if (DECL_CLONED_FUNCTION_P (decl)) - maybe_optimize_cdtor (decl); } /* Return an expression that performs the destruction of DECL, which @@ -5331,7 +5335,7 @@ c_parse_final_cleanups (void) node = node->get_alias_target (); node->call_for_symbol_thunks_and_aliases (clear_decl_external, - NULL, true); + NULL, true); /* If we mark !DECL_EXTERNAL one of the symbols in some comdat group, we need to mark all symbols in the same comdat group that way. */ @@ -5341,7 +5345,7 @@ c_parse_final_cleanups (void) next != node; next = dyn_cast (next->same_comdat_group)) next->call_for_symbol_thunks_and_aliases (clear_decl_external, - NULL, true); + NULL, true); } /* If we're going to need to write this function out, and --- gcc/cp/optimize.cc.jj 2024-04-25 20:33:30.771858912 +0200 +++ gcc/cp/optimize.cc 2024-05-09 17:10:23.920478922 +0200 @@ -220,10 +220,8 @@ can_alias_cdtor (tree fn) gcc_assert (DECL_MAYBE_IN_CHARGE_CDTOR_P (fn)); /* Don't use aliases for weak/linkonce definitions unless we can put both symbols in the same COMDAT group. */ - return (DECL_INTERFACE_KNOWN (fn) - && (SUPPORTS_ONE_ONLY || !DECL_WEAK (fn)) - && (!DECL_ONE_ONLY (fn) - || (HAVE_COMDAT_GROUP && DECL_WEAK (fn)))); + return (DECL_WEAK (fn) ? (HAVE_COMDAT_GROUP && DECL_ONE_ONLY (fn)) + : (DECL_INTERFACE_KNOWN (fn) && !DECL_ONE_ONLY (fn))); } /* FN is a [cd]tor, fns is a pointer to an array of length 3. Fill fns @@ -712,7 +710,7 @@ maybe_clone_body (tree fn) if (expand_or_defer_fn_1 (clone)) emit_associated_thunks (clone); /* We didn't generate a body, so remove the empty one. */ - DECL_SAVED_TREE (clone) = NULL_TREE; + DECL_SAVED_TREE (clone) = void_node; } else expand_or_defer_fn (clone); @@ -723,58 +721,3 @@ maybe_clone_body (tree fn) /* We don't need to process the original function any further. */ return 1; } - -/* If maybe_clone_body is called while the cdtor is still tentative, - DECL_ONE_ONLY will be false and so will be can_alias_cdtor (fn). - In that case we wouldn't try to optimize using an alias and instead - would emit separate base and complete cdtor. The following function - attempts to still optimize that case when we import_export_decl - is called first time on one of the clones. */ - -void -maybe_optimize_cdtor (tree orig_decl) -{ - tree fns[3]; - tree fn = DECL_CLONED_FUNCTION (orig_decl); - gcc_checking_assert (DECL_MAYBE_IN_CHARGE_CDTOR_P (fn)); - if (DECL_INTERFACE_KNOWN (fn) - || !TREE_ASM_WRITTEN (fn) - || !DECL_ONE_ONLY (orig_decl) - || symtab->global_info_ready) - return; - - populate_clone_array (fn, fns); - - if (!fns[0] || !fns[1]) - return; - for (int i = 2 - !fns[2]; i >= 0; --i) - if (fns[i] != orig_decl && DECL_INTERFACE_KNOWN (fns[i])) - return; - DECL_INTERFACE_KNOWN (fn) = 1; - comdat_linkage (fn); - if (!can_alias_cdtor (fn)) - return; - /* For comdat base and complete cdtors put them into the same, - *[CD]5* comdat group instead of *[CD][12]*. */ - auto n0 = cgraph_node::get_create (fns[0]); - auto n1 = cgraph_node::get_create (fns[1]); - auto n2 = fns[2] ? cgraph_node::get_create (fns[1]) : NULL; - if (n0->lowered || n1->lowered || (n2 && n2->lowered)) - return; - import_export_decl (fns[0]); - n1->definition = false; - if (!n0->create_same_body_alias (fns[1], fns[0])) - return; - - tree comdat_group = cdtor_comdat_group (fns[1], fns[0]); - n1 = cgraph_node::get (fns[1]); - n0->set_comdat_group (comdat_group); - if (n1->same_comdat_group) - n1->remove_from_same_comdat_group (); - n1->add_to_same_comdat_group (n0); - if (fns[2]) - n2->add_to_same_comdat_group (n0); - import_export_decl (fns[1]); - /* Remove the body now that it is an alias. */ - release_function_body (fns[1]); -} --- gcc/cp/decl.cc.jj 2024-05-09 10:30:54.804505130 +0200 +++ gcc/cp/decl.cc 2024-05-09 17:07:08.400110018 +0200 @@ -19280,6 +19280,14 @@ cxx_comdat_group (tree decl) else break; } + /* If a ctor/dtor has already set the comdat group by + maybe_clone_body, don't override it. */ + if (SUPPORTS_ONE_ONLY + && TREE_CODE (decl) == FUNCTION_DECL + && DECL_CLONED_FUNCTION_P (decl) + && SUPPORTS_ONE_ONLY) + if (tree comdat = DECL_COMDAT_GROUP (decl)) + return comdat; } return decl; --- gcc/testsuite/g++.dg/abi/comdat3.C.jj 2024-05-09 17:07:27.345855068 +0200 +++ gcc/testsuite/g++.dg/abi/comdat3.C 2024-05-09 17:07:08.400110018 +0200 @@ -0,0 +1,22 @@ +// PR lto/113208 +// { dg-do compile { target { c++11 && { *-*-*gnu* } } } } +// { dg-additional-options "-O2" } +// { dg-final { scan-assembler "_ZN1M1SINS_1P1TELN1N1LE2EEC5Ev,comdat" } } +// { dg-final { scan-assembler-not "_ZN1M1SINS_1P1TELN1N1LE2EEC1Ev,comdat" } } +// { dg-final { scan-assembler-not "_ZN1M1SINS_1P1TELN1N1LE2EEC2Ev,comdat" } } + +namespace N { enum L { L1, L2, L3 } const O = L3; } +namespace M { + using N::O; + using N::L; + template + struct S { constexpr S () {} }; + namespace P { + struct T; + struct U { S u; }; + void foo () { U (); } + } + extern template class S; +} +namespace p = M::P; +template class M::S; --- gcc/testsuite/g++.dg/abi/comdat4.C.jj 2024-05-09 17:07:31.961792950 +0200 +++ gcc/testsuite/g++.dg/abi/comdat4.C 2024-05-09 17:07:08.400110018 +0200 @@ -0,0 +1,28 @@ +// PR lto/113208 +// { dg-do compile { target { c++11 && { *-*-*gnu* } } } } +// { dg-additional-options "-O2" } +// { dg-final { scan-assembler "_ZN1BI1CEC5ERKS1_,comdat" } } +// { dg-final { scan-assembler-not "_ZN1BI1CEC1ERKS1_,comdat" } } +// { dg-final { scan-assembler-not "_ZN1BI1CEC2ERKS1_,comdat" } } + +template +struct A { + int foo () const; + A (int, int); +}; +template +struct B : A { + constexpr B (const B &x) : A (1, x.foo ()) {} + B () : A (1, 2) {} +}; +struct C; +struct D : B {}; +void bar (D); + +void +baz (D x) +{ + bar (x); +} + +template struct B; Jakub