On Sat, 12 Apr 2025, Jason Merrill wrote:
> On 4/11/25 4:36 PM, Patrick Palka wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > GCC 15 perhaps?
> >
> > -- >8 --
> >
> > Here checking
> >
> > static_assert(0 == big_calc());
> >
> > takes twice as much time as
> >
> > constexpr int ret = big_calc();
> > static_assert(0 == ret);
> >
> > ultimately because in the former, we first constexpr evaluate big_calc()
> > with mce_unknown (as part of warning-dependent folding from
> > cp_build_binary_op). We then constant evaluate it a second time, with
> > mce_true, during finish_static_assert. The result of the first
> > evaluation isn't reused because of the different mce_value.
> >
> > But the call doesn't depend on mce_value at all here (i.e. there's no if
> > consteval or __builtin_is_constant_evaluated calls, nested or otherwise)
> > so we should be able to reuse the result in this case. Specifically if a
> > constexpr call with mce_unknown gets successfully evaluated, we can safely
> > reuse the result during a subsequent mce_true or mce_false evaluation.
> >
> > This patch implements this by caching a successful mce_unknown call
> > result into the corresponding mce_true and mce_false slots, so that
> > such a subsequent evaluation effectively reuses the mce_unknown result.
> > To make it more convenient to access the cache slot for the same call
> > with different mce_value, this patch gives each constexpr_call entry
> > three result slots, one per mce_value, instead of having a distinct
> > constexpr_call entry for each mce_value. And we can no longer use
> > NULL_TREE as the "in progress" result, so instead use unknown_type_node.
> >
> > After this patch compile time for the above two fragments is the same.
> >
> > PR c++/115639
> >
> > gcc/cp/ChangeLog:
> >
> > * constexpr.cc (struct constexpr_call): Add NSDMIs to each
> > field. Replace 'result' data member with 3-element 'results'
> > array and a 'result' accessor function. Remove
> > 'manifestly_const_eval' data member.
> > (constexpr_call_hasher::equal): Adjust after constexpr_call
> > layout change.
> > (cxx_eval_call_expression): Likewise. Define some local
> > variables closer to their first use. Use unknown_type_node
> > instead of NULL_TREE as the "in progress" result. After
> > successully evaluating a call with mce_unknown, also cache the
> > result in the corresponding mce_true and mce_false slots.
> > ---
> > gcc/cp/constexpr.cc | 59 +++++++++++++++++++++++++++------------------
> > 1 file changed, 35 insertions(+), 24 deletions(-)
> >
> > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > index 0242425370f4..a4fd0d072f65 100644
> > --- a/gcc/cp/constexpr.cc
> > +++ b/gcc/cp/constexpr.cc
> > @@ -1119,20 +1119,22 @@ explain_invalid_constexpr_fn (tree fun)
> > struct GTY((for_user)) constexpr_call {
> > /* Description of the constexpr function definition. */
> > - constexpr_fundef *fundef;
> > + constexpr_fundef *fundef = nullptr;
> > /* Parameter bindings environment. A TREE_VEC of arguments. */
> > - tree bindings;
> > - /* Result of the call.
> > - NULL means the call is being evaluated.
> > + tree bindings = NULL_TREE;
> > + /* Result of the call, indexed by the value of
> > + constexpr_ctx::manifestly_const_eval.
> > + unknown_type_node means the call is being evaluated.
> > error_mark_node means that the evaluation was erroneous or
> > otherwise
> > uncacheable (e.g. because it depends on the caller).
> > Otherwise, the actual value of the call. */
> > - tree result;
> > + tree results[3] = { NULL_TREE, NULL_TREE, NULL_TREE };
> > /* The hash of this call; we remember it here to avoid having to
> > recalculate it when expanding the hash table. */
> > - hashval_t hash;
> > - /* The value of constexpr_ctx::manifestly_const_eval. */
> > - enum mce_value manifestly_const_eval;
> > + hashval_t hash = 0;
> > +
> > + /* The result slot corresponding to the given mce_value. */
> > + tree& result (mce_value mce) { return results[1 + int(mce)]; }
> > };
> > struct constexpr_call_hasher : ggc_ptr_hash<constexpr_call>
> > @@ -1427,8 +1429,6 @@ constexpr_call_hasher::equal (constexpr_call *lhs,
> > constexpr_call *rhs)
> > return true;
> > if (lhs->hash != rhs->hash)
> > return false;
> > - if (lhs->manifestly_const_eval != rhs->manifestly_const_eval)
> > - return false;
> > if (!constexpr_fundef_hasher::equal (lhs->fundef, rhs->fundef))
> > return false;
> > return cp_tree_equal (lhs->bindings, rhs->bindings);
> > @@ -2855,9 +2855,6 @@ cxx_eval_call_expression (const constexpr_ctx *ctx,
> > tree t,
> > {
> > location_t loc = cp_expr_loc_or_input_loc (t);
> > tree fun = get_function_named_in_call (t);
> > - constexpr_call new_call
> > - = { NULL, NULL, NULL, 0, ctx->manifestly_const_eval };
> > - int depth_ok;
> > if (fun == NULL_TREE)
> > return cxx_eval_internal_function (ctx, t, lval,
> > @@ -3099,7 +3096,6 @@ cxx_eval_call_expression (const constexpr_ctx *ctx,
> > tree t,
> > if (DECL_IMMEDIATE_FUNCTION_P (fun))
> > {
> > new_ctx.manifestly_const_eval = mce_true;
> > - new_call.manifestly_const_eval = mce_true;
> > ctx = &new_ctx;
> > }
> > @@ -3112,6 +3108,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx,
> > tree t,
> > || cp_noexcept_operand);
> > bool non_constant_args = false;
> > + constexpr_call new_call;
> > new_call.bindings
> > = cxx_bind_parameters_in_call (ctx, t, fun, non_constant_p,
> > overflow_p, &non_constant_args);
> > @@ -3185,7 +3182,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx,
> > tree t,
> > }
> > }
> > - depth_ok = push_cx_call_context (t);
> > + int depth_ok = push_cx_call_context (t);
> > /* Remember the object we are constructing or destructing. */
> > tree new_obj = NULL_TREE;
> > @@ -3227,8 +3224,6 @@ cxx_eval_call_expression (const constexpr_ctx *ctx,
> > tree t,
> > new_call.hash = constexpr_fundef_hasher::hash (new_call.fundef);
> > new_call.hash
> > = iterative_hash_template_arg (new_call.bindings, new_call.hash);
> > - new_call.hash
> > - = iterative_hash_object (ctx->manifestly_const_eval, new_call.hash);
> > /* If we have seen this call before, we are done. */
> > maybe_initialize_constexpr_call_table ();
> > @@ -3246,22 +3241,23 @@ cxx_eval_call_expression (const constexpr_ctx *ctx,
> > tree t,
> > the slot can move during evaluation of the body. */
> > *slot = entry = ggc_alloc<constexpr_call> ();
> > *entry = new_call;
> > + entry->result (ctx->manifestly_const_eval) = unknown_type_node;
> > fb.preserve ();
> > }
> > }
> > - /* Calls that are in progress have their result set to NULL, so that
> > we
> > - can detect circular dependencies. Now that we only cache up to
> > - constexpr_cache_depth this won't catch circular dependencies that
> > + /* Calls that are in progress have their result set to
> > unknown_type_node,
> > + so that we can detect circular dependencies. Now that we only cache
> > + up to constexpr_cache_depth this won't catch circular dependencies
> > that
> > start deeper, but they'll hit the recursion or ops limit. */
> > - else if (entry->result == NULL)
> > + else if (entry->result (ctx->manifestly_const_eval) ==
> > unknown_type_node)
> > {
> > if (!ctx->quiet)
> > error ("call has circular dependency");
> > *non_constant_p = true;
> > - entry->result = result = error_mark_node;
> > + entry->result (ctx->manifestly_const_eval) = result =
> > error_mark_node;
> > }
> > else
> > - result = entry->result;
> > + result = entry->result (ctx->manifestly_const_eval);
> > }
> > if (!depth_ok)
> > @@ -3482,7 +3478,22 @@ cxx_eval_call_expression (const constexpr_ctx *ctx,
> > tree t,
> > else if (!result)
> > result = void_node;
> > if (entry)
> > - entry->result = cacheable ? result : error_mark_node;
> > + {
> > + entry->result (ctx->manifestly_const_eval)
> > + = cacheable ? result : error_mark_node;
> > +
> > + if (result != error_mark_node
> > + && ctx->manifestly_const_eval == mce_unknown)
>
> This looks like it will set the true/false results to the actual result even
> if we set the unknown result to error_mark_node because !cacheable?
The code path sets the true/false results by directly copying the
current (i.e. mce_unknown) result:
entry->result (mce_true) = entry->result (mce_unknown);
entry->result (mce_false) = entry->result (mce_unknown);
so it'll mirror whatever logic is used to set the unknow result.
>
> > + {
> > + /* Evaluation succeeded and was independent of whether we're in
> > a
> > + manifestly constant-evaluated context, so we can also reuse
> > + this result with a fixed context. */
> > + if (!entry->result (mce_true))
> > + entry->result (mce_true) = entry->result (mce_unknown);
> > + if (!entry->result (mce_false))
> > + entry->result (mce_false) = entry->result (mce_unknown);
> > + }
> > + }
> > }
> > /* The result of a constexpr function must be completely initialized.
>
>
