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 9E31D3858405 for ; Thu, 28 Oct 2021 17:38:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9E31D3858405 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-163-k3HEKlVIN-u9sdgtuN6jew-1; Thu, 28 Oct 2021 13:38:32 -0400 X-MC-Unique: k3HEKlVIN-u9sdgtuN6jew-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 18BD2802682 for ; Thu, 28 Oct 2021 17:38:31 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.193.172]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A214F18368; Thu, 28 Oct 2021 17:38:21 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 19SHcJl72077122 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 28 Oct 2021 19:38:19 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 19SHcIRM2077121; Thu, 28 Oct 2021 19:38:18 +0200 Date: Thu, 28 Oct 2021 19:38:18 +0200 From: Jakub Jelinek To: Patrick Palka Cc: Jason Merrill , gcc-patches List Subject: Re: [PATCH] c++: quadratic constexpr behavior for left-assoc logical exprs [PR102780] Message-ID: <20211028173818.GU304296@tucnak> Reply-To: Jakub Jelinek References: <20211026174418.3142626-1-ppalka@redhat.com> <20211026184544.GA304296@tucnak> <99c81449-d68a-31c8-2ce6-fff154508c9@idea> <20211026211108.GE304296@tucnak> <40dcf6d9-a4a-227a-9553-43c13a42b28e@idea> <8f7e4b0e-0de6-b80c-387b-81ebfd340bab@redhat.com> <65132255-53fe-4c2c-a256-4f1deba260ad@idea> <2eb3439b-1e68-5664-ac2e-f85e891c2a1b@redhat.com> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 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=-5.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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, 28 Oct 2021 17:38:35 -0000 On Thu, Oct 28, 2021 at 12:40:02PM -0400, Patrick Palka wrote: > PR c++/102780 > > gcc/cp/ChangeLog: > > * constexpr.c (potential_constant_expression_1) : > When tf_error isn't set, preemptively check potentiality of the > second operand before performing trial evaluation of the first > operand. > (potential_constant_expression_1): When tf_error is set, first check > potentiality quietly and return true if successful, otherwise > proceed noisily to give errors. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp1z/fold13.C: New test. > --- > gcc/cp/constexpr.c | 26 +++++++++++++++++++++----- > gcc/testsuite/g++.dg/cpp1z/fold13.C | 29 +++++++++++++++++++++++++++++ > 2 files changed, 50 insertions(+), 5 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp1z/fold13.C Is there a reason to turn this mode of evaluating everything twice if an error should be diagnosed all the time, rather than only if we actually see a TRUTH_*_EXPR we want to handle this way? If we don't see any TRUTH_*_EXPR, or if processing_template_decl, or if the first operand is already a constant, that seems like a waste of time. So, can't we instead drop the second hunk, if processing_template_decl or TREE_CODE (op0) == INTEGER_CST do what the code did before, and just otherwise if flag & tf_error temporarily clear it, RECUR on op1 first, then cxx_eval_outermost_constant_expr and if tree_int_cst_equal RECUR on op1 the second time with tf_error and some extra flag (1 << 30?) that will mean not to RECUR on op1 twice in recursive invocations (to avoid bad compile time complexity on (x && (x && (x && (x && (x && (x && (x && (x && (x && (x && (x && (x && y)))))))))))) etc. if x constant evaluates to true and y is not potential constant expression. Though, I'm not sure that doing the RECUR (op1, rval) instead of cxx_eval_outermost_constant_expr must be generally a win for compile time, it can be if op0 is very large expression, but it can be a pessimization if op1 is huge and op0 is simple and doesn't constexpr evaluate to tmp. As I said, another possibility is something like: /* Try to quietly evaluate T to constant, but don't try too hard. */ static tree potential_constant_expression_eval (tree t) { auto o = make_temp_override (constexpr_ops_limit, MIN (constexpr_ops_limit, 100)); return cxx_eval_outermost_constant_expr (t, true); } and using this new function instead of cxx_eval_outermost_constant_expr (op, true); everywhere in potential_constant_expression_1 should fix the quadraticness too. > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -8892,13 +8892,18 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, > tmp = boolean_false_node; > truth: > { > - tree op = TREE_OPERAND (t, 0); > - if (!RECUR (op, rval)) > + tree op0 = TREE_OPERAND (t, 0); > + tree op1 = TREE_OPERAND (t, 1); > + if (!RECUR (op0, rval)) > return false; > + if (!(flags & tf_error) && RECUR (op1, rval)) > + /* When quiet, try to avoid expensive trial evaluation by first > + checking potentiality of the second operand. */ > + return true; > if (!processing_template_decl) > - op = cxx_eval_outermost_constant_expr (op, true); > - if (tree_int_cst_equal (op, tmp)) > - return RECUR (TREE_OPERAND (t, 1), rval); > + op0 = cxx_eval_outermost_constant_expr (op0, true); > + if (tree_int_cst_equal (op0, tmp)) > + return (flags & tf_error) ? RECUR (op1, rval) : false; > else > return true; > } > @@ -9107,6 +9112,17 @@ bool > potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, > tsubst_flags_t flags) > { > + if (flags & tf_error) > + { > + /* Check potentiality quietly first, as that could be performed more > + efficiently in some cases (currently only for TRUTH_*_EXPR). If > + that fails, replay the check noisily to give errors. */ > + flags &= ~tf_error; > + if (potential_constant_expression_1 (t, want_rval, strict, now, flags)) > + return true; > + flags |= tf_error; > + } > + > tree target = NULL_TREE; > return potential_constant_expression_1 (t, want_rval, strict, now, > flags, &target); Jakub