On Wed, 27 Oct 2021, Jason Merrill wrote: > On 10/27/21 14:54, Patrick Palka wrote: > > On Tue, 26 Oct 2021, Jakub Jelinek wrote: > > > > > On Tue, Oct 26, 2021 at 05:07:43PM -0400, Patrick Palka wrote: > > > > The performance impact of the other calls to > > > > cxx_eval_outermost_const_expr > > > > from p_c_e_1 is probably already mostly mitigated by the constexpr call > > > > cache and the fact that we try to evaluate all calls to constexpr > > > > functions during cp_fold_function anyway (at least with -O). So trial > > > > > > constexpr function bodies don't go through cp_fold_function > > > (intentionally, > > > so that we don't optimize away UB), the bodies are copied before the trees > > > of the > > > normal copy are folded. > > > > Ah right, I had forgotten about that.. > > > > Here's another approach that doesn't need to remove trial evaluation for > > &&/||. The idea is to first quietly check if the second operand is > > potentially constant _before_ performing trial evaluation of the first > > operand. This speeds up the case we care about (both operands are > > potentially constant) without regressing any diagnostics. We have to be > > careful about emitting bogus diagnostics when tf_error is set, hence the > > first hunk below which makes p_c_e_1 always proceed quietly first, and > > replay noisily in case of error (similar to how satisfaction works). > > > > Would something like this be preferable? > > Seems plausible, though doubling the number of stack frames is a downside.
Whoops, good point.. The noisy -> quiet adjustment only needs to be performed during the outermost call to p_c_e_1, and not also during each recursive call. The revised diff below fixes this thinko, and so only a single extra stack frame is needed AFAICT. > > What did you think of Jakub's suggestion of linearizing the terms? IIUC that would fix the quadraticness, but it wouldn't address that we end up evaluating the entire expression twice, once during the trial evaluation of each term from p_c_e_1 and again during the proper evaluation of the entire expression. It'd be nice if we could somehow avoid the double evaluation, as in the approach below (or in the first patch). -- >8 -- gcc/cp/ChangeLog: * constexpr.c (potential_constant_expression_1): When tf_error is set, proceed quietly first and return true if successful. <case TRUTH_*_EXPR>: When tf_error is not set, check potentiality of the second operand before performing trial evaluation of the first operand rather than after. diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 6f83d303cdd..7855a948baf 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -8892,13 +8892,16 @@ 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)) + 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 +9110,14 @@ bool potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, tsubst_flags_t flags) { + if (flags & tf_error) + { + 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);