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 09779382E915 for ; Thu, 9 Jun 2022 19:16:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 09779382E915 Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-204-3uFywrCaPzav8e1CZjYiUQ-1; Thu, 09 Jun 2022 15:16:12 -0400 X-MC-Unique: 3uFywrCaPzav8e1CZjYiUQ-1 Received: by mail-qt1-f200.google.com with SMTP id t35-20020a05622a182300b00305099b873bso3169868qtc.18 for ; Thu, 09 Jun 2022 12:16:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:date:to:cc:subject:in-reply-to:message-id :references:mime-version; bh=YFmGxGKeF9mzdCA9y0sR8QrSwZK2tLQ0G9FM5RHWZCc=; b=U6NpNIvuPIpVqgsFlXeuukXGrgWwrEpbSBvfTA716SXrlSm4BTJ4+pSX9cQkLaQsuA ovAb8qNlblNwGGy2wo7NZVtQ1S/MAdOgNSrzPmj3UeJZlNG96ahjx9xahRHcNpBzWmha CpIkKwIH774MjqdbQ3z/cspcG40VoRXtk6Soksezhs2PCjTqeHjzTLOaceiGvPTk6Hal R1wLOgAPRoFmZYTsCyu9JuyJ++cyQVIHukxULCpM05n/kfSG4WXtu88h6dfWky97NBj6 cseXY4G+YMKV7VFRLAFw3AFQO2XvvJnnKtmRLxdJ8S9yElHZ/Bd0EV0MYnTWYFSDLs5p ZJ3w== X-Gm-Message-State: AOAM533lUa0+IEV0TGLnUzDXP+sFXKb1hqxRq4+ZpZIqzDIV1rZ6KVxE 8MNLHOFBQBTnnVSKqgbpn7j/24ACyFsrfTjrATC3YDHSm8aZWaxyeEzZI76qeYbnhb80dmo9OBv UY8oHnLKjku7D8sLXFQ== X-Received: by 2002:a05:6214:d05:b0:464:57fe:17b7 with SMTP id 5-20020a0562140d0500b0046457fe17b7mr42166553qvh.13.1654802171916; Thu, 09 Jun 2022 12:16:11 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz/rYqEk3I1ZKDXnsGHoZ7KURkD/42HWWm7XvU7HeVnsLrClbraOVBVVYumtbS7x2a+vm0AZA== X-Received: by 2002:a05:6214:d05:b0:464:57fe:17b7 with SMTP id 5-20020a0562140d0500b0046457fe17b7mr42166516qvh.13.1654802171440; Thu, 09 Jun 2022 12:16:11 -0700 (PDT) Received: from [192.168.1.130] (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id b26-20020a05620a271a00b0069fc13ce1e9sm2826864qkp.26.2022.06.09.12.16.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Jun 2022 12:16:11 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Thu, 9 Jun 2022 15:16:10 -0400 (EDT) To: Jason Merrill cc: Patrick Palka , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c++: optimize specialization of nested class templates In-Reply-To: Message-ID: References: <20220608182147.4123587-1-ppalka@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-14.7 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, 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.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Jun 2022 19:16:15 -0000 On Thu, 9 Jun 2022, Jason Merrill wrote: > On 6/8/22 14:21, Patrick Palka wrote: > > When substituting a class template specialization, tsubst_aggr_type > > substitutes the TYPE_CONTEXT before passing it to lookup_template_class. > > This appears to be unnecessary, however, because the the initial value > > of lookup_template_class's context parameter is unused outside of the > > IDENTIFIER_NODE case, and l_t_c performs its own substitution of the > > context, anyway. So this patch removes the redundant substitution in > > tsubst_aggr_type. Doing so causes us to ICE on template/nested5.C > > because during lookup_template_class for A::C::D with T=E and S=S, > > we substitute and complete the context A::C with T=E, which in turn > > registers the desired dependent specialization of D for us and we end up > > trying to register it again. This patch fixes this by checking the > > specializations table again after completion of the context. > > > > This patch also implements a couple of other optimizations: > > > > * In lookup_template_class, if the context of the partially > > instantiated template is already non-dependent, then we could > > reuse that instead of substituting the context of the most > > general template. > > * When substituting the TYPE_DECL for an injected-class-name > > in tsubst_decl, we can avoid substituting its TREE_TYPE and > > DECL_TI_ARGS. > > > > Together these optimizations improve memory usage for the range-v3 > > testcase test/view/split.cc by about 5%. The improvement is probably > > more significant when dealing with deeply nested class templates. > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > > trunk? > > > > gcc/cp/ChangeLog: > > > > * pt.cc (lookup_template_class): Remove dead stores to > > context parameter. Don't substitute the context of the > > most general template if that of the partially instantiated > > template is non-dependent. Check the specializations table > > again after completing the context of a nested dependent > > specialization. > > (tsubst_aggr_type) : Don't substitute > > TYPE_CONTEXT or pass it to lookup_template_class. > > (tsubst_decl) : Avoid substituting the > > TREE_TYPE and DECL_TI_ARGS when DECL_SELF_REFERENCE_P. > > --- > > gcc/cp/pt.cc | 69 +++++++++++++++++++++++++++++++--------------------- > > 1 file changed, 41 insertions(+), 28 deletions(-) > > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > > index 59b94317e88..28023d60684 100644 > > --- a/gcc/cp/pt.cc > > +++ b/gcc/cp/pt.cc > > @@ -9840,8 +9840,6 @@ lookup_template_class (tree d1, tree arglist, tree > > in_decl, tree context, > > if (context) > > pop_decl_namespace (); > > } > > - if (templ) > > - context = DECL_CONTEXT (templ); > > } > > else if (TREE_CODE (d1) == TYPE_DECL && MAYBE_CLASS_TYPE_P (TREE_TYPE > > (d1))) > > { > > @@ -9868,7 +9866,6 @@ lookup_template_class (tree d1, tree arglist, tree > > in_decl, tree context, > > { > > templ = d1; > > d1 = DECL_NAME (templ); > > - context = DECL_CONTEXT (templ); > > } > > else if (DECL_TEMPLATE_TEMPLATE_PARM_P (d1)) > > { > > @@ -10059,8 +10056,25 @@ lookup_template_class (tree d1, tree arglist, tree > > in_decl, tree context, > > context = DECL_CONTEXT (gen_tmpl); > > if (context && TYPE_P (context)) > > { > > - context = tsubst_aggr_type (context, arglist, complain, in_decl, > > true); > > - context = complete_type (context); > > + if (!uses_template_parms (DECL_CONTEXT (templ))) > > + /* If the context of the partially instantiated template is > > + already non-dependent, then we might as well use it. */ > > + context = DECL_CONTEXT (templ); > > + else > > + { > > + context = tsubst_aggr_type (context, arglist, complain, in_decl, > > true); > > + context = complete_type (context); > > + if (is_dependent_type && arg_depth > 1) > > + { > > + /* If this is a dependent nested specialization such as > > + A::B, then completion of A might have > > + registered this specialization of B for us, so check > > + the table again (33959). */ > > + entry = type_specializations->find_with_hash (&elt, hash); > > + if (entry) > > + return entry->spec; > > + } > > + } > > } > > else > > context = tsubst (context, arglist, complain, in_decl); > > @@ -13711,25 +13725,12 @@ tsubst_aggr_type (tree t, > > if (TYPE_TEMPLATE_INFO (t) && uses_template_parms (t)) > > { > > tree argvec; > > - tree context; > > tree r; > > /* In "sizeof(X)" we need to evaluate "I". */ > > cp_evaluated ev; > > - /* First, determine the context for the type we are looking > > - up. */ > > - context = TYPE_CONTEXT (t); > > - if (context && TYPE_P (context)) > > - { > > - context = tsubst_aggr_type (context, args, complain, > > - in_decl, /*entering_scope=*/1); > > - /* If context is a nested class inside a class template, > > - it may still need to be instantiated (c++/33959). */ > > - context = complete_type (context); > > - } > > - > > - /* Then, figure out what arguments are appropriate for the > > + /* Figure out what arguments are appropriate for the > > type we are trying to find. For example, given: > > template struct S; > > @@ -13744,7 +13745,7 @@ tsubst_aggr_type (tree t, > > r = error_mark_node; > > else > > { > > - r = lookup_template_class (t, argvec, in_decl, context, > > + r = lookup_template_class (t, argvec, in_decl, NULL_TREE, > > entering_scope, complain); > > r = cp_build_qualified_type (r, cp_type_quals (t), complain); > > } > > @@ -14880,6 +14881,10 @@ tsubst_decl (tree t, tree args, tsubst_flags_t > > complain) > > ctx = tsubst_aggr_type (ctx, args, > > complain, > > in_decl, /*entering_scope=*/1); > > + if (DECL_SELF_REFERENCE_P (t)) > > + /* The context and type of a injected-class-name are > > + the same, so we don't need to substitute both. */ > > + type = ctx; > > /* If CTX is unchanged, then T is in fact the > > specialization we want. That situation occurs when > > referencing a static data member within in its own > > @@ -14900,14 +14905,22 @@ tsubst_decl (tree t, tree args, tsubst_flags_t > > complain) > > { > > tmpl = DECL_TI_TEMPLATE (t); > > gen_tmpl = most_general_template (tmpl); > > - argvec = tsubst (DECL_TI_ARGS (t), args, complain, in_decl); > > - if (argvec != error_mark_node) > > - argvec = (coerce_innermost_template_parms > > - (DECL_TEMPLATE_PARMS (gen_tmpl), > > - argvec, t, complain, > > - /*all*/true, /*defarg*/true)); > > - if (argvec == error_mark_node) > > - RETURN (error_mark_node); > > + if (DECL_SELF_REFERENCE_P (t)) > > + /* The DECL_TI_ARGS for the injected-class-name are the > > + generic template arguments for the class template, so > > + substitution/coercion is just the identity mapping. */ > > + argvec = args; > > Would it make sense to extend this to any TEMPLATE_DECL for which > DECL_PRIMARY_TEMPLATE is the class template? So, anything that gets here > except an alias template. Ah nice, it does look like we could extend this to any TEMPLATE_DECL that satisfies DECL_CLASS_SCOPE_P && !DECL_MEMBER_TEMPLATE_P (so that we also exclude static data member templates), i.e. any templated non-template member. Is that equivalent to what you had in mind? Based on some light testing, if we do this, it seems we also need to handle 'args' having greater depth than 'DECL_TI_ARGS' here, something which could happen during satisfaction. I can work on extending this as a followup patch. > > > + else > > + { > > + argvec = tsubst (DECL_TI_ARGS (t), args, complain, > > in_decl); > > + if (argvec != error_mark_node) > > + argvec = (coerce_innermost_template_parms > > + (DECL_TEMPLATE_PARMS (gen_tmpl), > > + argvec, t, complain, > > + /*all*/true, /*defarg*/true)); > > + if (argvec == error_mark_node) > > + RETURN (error_mark_node); > > + } > > hash = hash_tmpl_and_args (gen_tmpl, argvec); > > spec = retrieve_specialization (gen_tmpl, argvec, hash); > > } > >