From patchwork Thu Oct 6 02:02:26 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Palka X-Patchwork-Id: 1686558 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=e9v22Zf5; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4MjZTY6dVfz20Pd for ; Thu, 6 Oct 2022 13:02:57 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id BF05C384D154 for ; Thu, 6 Oct 2022 02:02:55 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BF05C384D154 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1665021775; bh=LwNIBao7g61x1Y5ic9vZNtriyN8sHTown/+GyviSg9k=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=e9v22Zf5EcU4X+EfP0PtbavN6IqSgzsDyqZSZn2W9iNUfPh81Qe5+SZYDCuk1uTRI 5tu8qk8Y7OA6gJqFzzWuotnrlB9yVI8+rUYYgKOwASndK1vEPw8K6kfGKoetgJ1giZ PCzdqpY/yQBD+MDSnW2h6hq5GrMFcqqJN/INIYzc= 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.129.124]) by sourceware.org (Postfix) with ESMTPS id C25A23853835 for ; Thu, 6 Oct 2022 02:02:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C25A23853835 Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-155-ZF6zZcvvOQCVyRTBj5FCyg-1; Wed, 05 Oct 2022 22:02:31 -0400 X-MC-Unique: ZF6zZcvvOQCVyRTBj5FCyg-1 Received: by mail-qt1-f197.google.com with SMTP id w4-20020a05622a134400b0035cbc5ec9a2so289118qtk.14 for ; Wed, 05 Oct 2022 19:02:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=LwNIBao7g61x1Y5ic9vZNtriyN8sHTown/+GyviSg9k=; b=An0+78ERnQcV0/ZhVp+j4A6rQL3S/IbzkLszWYzaeJeIhrtoh/6mn2RnXqEdq+hB72 +8fUPjeIppfnWfGPeqFNaO+cPv1WMcWQ5A7TN47QYNv5UBw198NJvoXC7uqaNPVc9F7i DYq/HubMiKn3NZQ7LbJV6uh9MZTt0rWAD0GesyYc/41T0fMwIupUSEAgi3O5xbIHRmT+ RiPxKPFc3PDliEGZFb6zZWR1b45GoW94W3+T7FKOKnOcLT3zkQiPY63NQawfareurI9K wnH4dWz9aYCyuwXbvZeZRpOOgojmVM5txlYVrYfAuogB8R9plPDRMpoWSsqU/+yJwX+B e/uw== X-Gm-Message-State: ACrzQf3efLp+Kk5ySalmOEICOf1ZD6I38Uf1T9nlmZtl3ff7i+8SM3EX sfzwWTEI3OFBWkPlJpoFN+HtDGoAyhbjyHcsgKo+870dBUdIWHZkj40n+bpS+hTN+Sv0tPsWR6M p11vQjAud6gBTBZjgzPTJVdX4ciJSanuczu4zbuBv8Wi2B7MyiM/wXIVwieGnOGDmFIg= X-Received: by 2002:a05:620a:4726:b0:6ce:9ea8:4c23 with SMTP id bs38-20020a05620a472600b006ce9ea84c23mr1712058qkb.127.1665021750122; Wed, 05 Oct 2022 19:02:30 -0700 (PDT) X-Google-Smtp-Source: AMsMyM489MnXxUrO4AegUVX09Dfi5IsjoYyrTgUiQVIGg8EP9SN81lHgit8cM0irum5y9SYhsxM3PA== X-Received: by 2002:a05:620a:4726:b0:6ce:9ea8:4c23 with SMTP id bs38-20020a05620a472600b006ce9ea84c23mr1712024qkb.127.1665021749650; Wed, 05 Oct 2022 19:02:29 -0700 (PDT) Received: from localhost.localdomain (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id t39-20020a05622a182700b0039442ee69c5sm351412qtc.91.2022.10.05.19.02.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Oct 2022 19:02:29 -0700 (PDT) To: gcc-patches@gcc.gnu.org Subject: [PATCH] c++: remove optimize_specialization_lookup_p Date: Wed, 5 Oct 2022 22:02:26 -0400 Message-Id: <20221006020226.3629040-1-ppalka@redhat.com> X-Mailer: git-send-email 2.38.0.rc2 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-14.3 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_LOW, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Patrick Palka via Gcc-patches From: Patrick Palka Reply-To: Patrick Palka Cc: nathan@acm.org Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" Roughly speaking, optimize_specialization_lookup_p returns true for a non-template member function of a class template, e.g. template struct A { int f(); }; The idea behind the optimization in question seems to be that if we want to look up the specialization A::f [with T=int], then we can just do a name lookup for f in A and avoid having to maintain a spec_entry for it in the decl_specializations table. However, the benefit of this optimization seems questionable because in order to do the name lookup we need to look up A [with T=int] in the type_specializations table, which is just as expensive as looking up an entry in decl_specializations. And according to my experiments using stdc++.h, range-v3 and libstdc++ tests, the compiler is slightly (<1%) _faster_ after disabling this optimization! Additionally, this optimization means that an explicit specialization won't get recorded in decl_specializations for such a template, which is a rather arbitrary invariant violation, and apparently breaks the below modules testcase. So since this optimization doesn't seem to give a performance benefit, complicates the explicit specialization story, and affects modules, this patch proposes to remove it. Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk? Patch generated with -w to suppress noisy whitespace changes. gcc/cp/ChangeLog: * pt.cc (optimize_specialization_lookup_p): Remove. (retrieve_specialization): Assume the above returns false and simplify accordingly. (register_specialization): Likewise. gcc/testsuite/ChangeLog: * g++.dg/modules/indirect-3_b.C: Expect that the entity foo::TPL<0>::frob has is tagged as a specialization and not just a declaration. * g++.dg/modules/tpl-spec-8_a.H: New test. * g++.dg/modules/tpl-spec-8_b.C: New test. --- gcc/cp/pt.cc | 85 +-------------------- gcc/testsuite/g++.dg/modules/indirect-3_b.C | 2 +- gcc/testsuite/g++.dg/modules/tpl-spec-8_a.H | 9 +++ gcc/testsuite/g++.dg/modules/tpl-spec-8_b.C | 8 ++ 4 files changed, 22 insertions(+), 82 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/tpl-spec-8_a.H create mode 100644 gcc/testsuite/g++.dg/modules/tpl-spec-8_b.C diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index bce2a777922..c079bbd4268 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -1170,39 +1170,6 @@ maybe_process_partial_specialization (tree type) return type; } -/* Returns nonzero if we can optimize the retrieval of specializations - for TMPL, a TEMPLATE_DECL. In particular, for such a template, we - do not use DECL_TEMPLATE_SPECIALIZATIONS at all. */ - -static inline bool -optimize_specialization_lookup_p (tree tmpl) -{ - return (DECL_FUNCTION_TEMPLATE_P (tmpl) - && DECL_CLASS_SCOPE_P (tmpl) - /* DECL_CLASS_SCOPE_P holds of T::f even if T is a template - parameter. */ - && CLASS_TYPE_P (DECL_CONTEXT (tmpl)) - /* The optimized lookup depends on the fact that the - template arguments for the member function template apply - purely to the containing class, which is not true if the - containing class is an explicit or partial - specialization. */ - && !CLASSTYPE_TEMPLATE_SPECIALIZATION (DECL_CONTEXT (tmpl)) - && !DECL_MEMBER_TEMPLATE_P (tmpl) - && !DECL_CONV_FN_P (tmpl) - /* It is possible to have a template that is not a member - template and is not a member of a template class: - - template - struct S { friend A::f(); }; - - Here, the friend function is a template, but the context does - not have template information. The optimized lookup relies - on having ARGS be the template arguments for both the class - and the function template. */ - && !DECL_UNIQUE_FRIEND_P (DECL_TEMPLATE_RESULT (tmpl))); -} - /* Make sure ARGS doesn't use any inappropriate typedefs; we should have gone through coerce_template_parms by now. */ @@ -1276,43 +1243,12 @@ retrieve_specialization (tree tmpl, tree args, hashval_t hash) if (lambda_fn_in_template_p (tmpl)) return NULL_TREE; - if (optimize_specialization_lookup_p (tmpl)) - { - /* The template arguments actually apply to the containing - class. Find the class specialization with those - arguments. */ - tree class_template = CLASSTYPE_TI_TEMPLATE (DECL_CONTEXT (tmpl)); - tree class_specialization - = retrieve_specialization (class_template, args, 0); - if (!class_specialization) - return NULL_TREE; - - /* Find the instance of TMPL. */ - tree fns = get_class_binding (class_specialization, DECL_NAME (tmpl)); - for (ovl_iterator iter (fns); iter; ++iter) - { - tree fn = *iter; - if (tree ti = get_template_info (fn)) - if (TI_TEMPLATE (ti) == tmpl - /* using-declarations can bring in a different - instantiation of tmpl as a member of a different - instantiation of tmpl's class. We don't want those - here. */ - && DECL_CONTEXT (fn) == class_specialization) - return fn; - } - return NULL_TREE; - } - else - { - spec_entry *found; spec_entry elt; - spec_hash_table *specializations; - elt.tmpl = tmpl; elt.args = args; elt.spec = NULL_TREE; + spec_hash_table *specializations; if (DECL_CLASS_TEMPLATE_P (tmpl)) specializations = type_specializations; else @@ -1320,10 +1256,8 @@ retrieve_specialization (tree tmpl, tree args, hashval_t hash) if (hash == 0) hash = spec_hasher::hash (&elt); - found = specializations->find_with_hash (&elt, hash); - if (found) + if (spec_entry *found = specializations->find_with_hash (&elt, hash)) return found->spec; - } return NULL_TREE; } @@ -1567,8 +1501,6 @@ register_specialization (tree spec, tree tmpl, tree args, bool is_friend, hashval_t hash) { tree fn; - spec_entry **slot = NULL; - spec_entry elt; gcc_assert ((TREE_CODE (tmpl) == TEMPLATE_DECL && DECL_P (spec)) || (TREE_CODE (tmpl) == FIELD_DECL @@ -1589,12 +1521,7 @@ register_specialization (tree spec, tree tmpl, tree args, bool is_friend, instantiation unless and until it is actually needed. */ return spec; - if (optimize_specialization_lookup_p (tmpl)) - /* We don't put these specializations in the hash table, but we might - want to give an error about a mismatch. */ - fn = retrieve_specialization (tmpl, args, 0); - else - { + spec_entry elt; elt.tmpl = tmpl; elt.args = args; elt.spec = spec; @@ -1602,12 +1529,11 @@ register_specialization (tree spec, tree tmpl, tree args, bool is_friend, if (hash == 0) hash = spec_hasher::hash (&elt); - slot = decl_specializations->find_slot_with_hash (&elt, hash, INSERT); + spec_entry **slot = decl_specializations->find_slot_with_hash (&elt, hash, INSERT); if (*slot) fn = (*slot)->spec; else fn = NULL_TREE; - } /* We can sometimes try to re-register a specialization that we've already got. In particular, regenerate_decl_from_template calls @@ -1704,8 +1630,6 @@ register_specialization (tree spec, tree tmpl, tree args, bool is_friend, && !check_specialization_namespace (tmpl)) DECL_CONTEXT (spec) = DECL_CONTEXT (tmpl); - if (slot != NULL /* !optimize_specialization_lookup_p (tmpl) */) - { spec_entry *entry = ggc_alloc (); gcc_assert (tmpl && args && spec); *entry = elt; @@ -1723,7 +1647,6 @@ register_specialization (tree spec, tree tmpl, tree args, bool is_friend, specialization would have used it. */ DECL_TEMPLATE_INSTANTIATIONS (tmpl) = tree_cons (args, spec, DECL_TEMPLATE_INSTANTIATIONS (tmpl)); - } return spec; } diff --git a/gcc/testsuite/g++.dg/modules/indirect-3_b.C b/gcc/testsuite/g++.dg/modules/indirect-3_b.C index 5bdfc1d36a8..038b01ecab7 100644 --- a/gcc/testsuite/g++.dg/modules/indirect-3_b.C +++ b/gcc/testsuite/g++.dg/modules/indirect-3_b.C @@ -23,7 +23,7 @@ namespace bar // { dg-final { scan-lang-dump {Lazily binding '::foo@foo:.::TPL'@'foo' section} module } } // { dg-final { scan-lang-dump {Wrote import:-[0-9]* template_decl:'::foo@foo:.::template TPL@foo:.'@foo} module } } -// { dg-final { scan-lang-dump {Cluster members:\n \[0\]=specialization definition '::foo@foo:.::TPL<0x0>'\n \[1\]=specialization declaration '::foo@foo:.::TPL<0x0>::TPL<0x0>'\n( \[.\]=[^\n]* '\n)* \[.\]=decl definition '::foo@foo:.::TPL<0x0>::frob<0x0>'\n} module } } +// { dg-final { scan-lang-dump {Cluster members:\n \[0\]=specialization definition '::foo@foo:.::TPL<0x0>'\n \[1\]=specialization definition '::foo@foo:.::TPL<0x0>::frob<0x0>'\n \[2\]=specialization declaration '::foo@foo:.::TPL<0x0>::TPL<0x0>'} module } } // { dg-final { scan-lang-dump {Cluster members:\n \[0\]=specialization definition '::foo@foo:.::X@foo:.::frob<0x0>'} module } } // { dg-final { scan-lang-dump {Writing:-[0-9]*'s type spec merge key \(specialization\) type_decl:'::foo@foo:.::TPL<0x0>'} module } } diff --git a/gcc/testsuite/g++.dg/modules/tpl-spec-8_a.H b/gcc/testsuite/g++.dg/modules/tpl-spec-8_a.H new file mode 100644 index 00000000000..5f85556d7d7 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-spec-8_a.H @@ -0,0 +1,9 @@ +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } + +template +struct A { + static void f() { T::nonexistent; } +}; + +template<> inline void A::f() { } diff --git a/gcc/testsuite/g++.dg/modules/tpl-spec-8_b.C b/gcc/testsuite/g++.dg/modules/tpl-spec-8_b.C new file mode 100644 index 00000000000..f23eb370370 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-spec-8_b.C @@ -0,0 +1,8 @@ +// { dg-additional-options -fmodules-ts } +// { dg-do link } + +import "tpl-spec-8_a.H"; + +int main() { + A::f(); +}