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) <case TRUTH_*_EXPR>:
> 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