On Wed, Mar 12, 2025 at 05:39:46PM -0400, Jason Merrill wrote:
> On 3/7/25 11:54 AM, Jakub Jelinek wrote:
> > Hi!
> >
> > The r14-4140 change moved consteval evaluation from build_over_call to
> > cp_fold_r.
> >
> > The following testcase is a regression caused by that change. There
> > is a cast around immediate invocation, (int) foo (0x23)
> > where consteval for returns unsigned char. When the consteval call
> > has been folded early to 0x23 (with unsigned char type), cp_fold sees
> > (int) 0x23 and folds that to 0x23 with int type.
> > But when the immediate invocation is handled only during cp_fold_r,
> > cp_fold_r first calls cp_fold on the NOP_EXPR, which calls cp_fold
> > on its operand, it is CALL_EXPR, nothing is folded at that point.
> > Then cp_fold_r continues to walk the NOP_EXPR's operand, sees it is
> > an immediate function invocation and cp_fold_immediate_r calls
> > cxx_constant_value on it and replaces the CALL_EXPR with the INTEGER_CST
> > 0x23. Nothing comes back to folding the containing NOP_EXPR though.
> > Sure, with optimizations enabled some GIMPLE optimization folds that later,
> > but e.g. with -O0 nothing does that. I think there could be arbitrarily
> > complex expressions on top of the immediate invocation(s) that used to be
> > folded by cp_fold before and aren't folded anymore.
> >
> > One possibility would be to do the immediate invocation evaluation in
> > cp_fold rather than cp_fold_r (or in addition to cp_fold_r), but that
> > could mean we evaluate it multiple times and e.g. if there is an error
> > emit multiple errors.
> >
> > The following patch instead first evaluates all immediate invocations and
> > does cp_fold_r in a separate step. That not only allows the folding of
> > expressions which contain immediate invocations, but also simplifies some
> > of the quirks that had to be done when it was in cp_fold_r.
> > Though, I had to add an extra case to cp_genericize_r RETURN_EXPR handling
> > to avoid a regression where after emitting errors in RETURN_EXPR argument
> > we've emitted a -Wreturn-type false positive. Previously we ended up with
> > RETURN_EXPR with CLEANUP_POINT_EXPR with INIT_EXPR of RESULT_DECL to
> > error_mark_node, now we fold it more and have RETURN_EXPR with
> > error_mark_node operand. The former would result during gimplification
> > into something -Wresult-type was quiet about, the latter doesn't.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> > BTW, r14-4140 changed behavior on
> > consteval bool foo (bool x) { if (x) throw 1; return false; }
> >
> > constexpr void
> > foo ()
> > {
> > if constexpr (false)
> > {
> > bool a = foo (true);
> > }
> > }
> > where GCC 13 emitted
> > error: expression ‘<throw-expression>’ is not a constant expression
> > error and GCC 14/trunk including the patch below doesn't reject it.
> > And clang++ trunk rejects it. It isn't immediately clear to me what
> > is right, if immediate invocations in discarded statements should
> > be evaluated or not.
> >
> > 2025-03-07 Jakub Jelinek <ja...@redhat.com>
> >
> > PR target/118068
> > gcc/cp/
> > * cp-gimplify.cc (cp_fold_immediate): Use cp_walk_tree rather than
> > cp_walk_tree_without_duplicates.
> > (cp_fold_immediate_r): For IF_STMT_CONSTEVAL_P IF_STMT don't walk
> > into THEN_CLAUSE subtree, only ELSE_CLAUSE. For non-call related
> > stmts call data->pset.add and if it returns true, don't walk subtrees.
> > (cp_fold_r): Don't call cp_fold_immediate_r here.
> > (cp_fold_function): For C++20 or later call cp_walk_tree
> > with cp_fold_immediate_r callback first before calling cp_walk_tree
> > with cp_fold_r callback and call data.pset.empty () in between.
> > (cp_fully_fold_init): Likewise.
> > (cp_genericize_r) <case RETURN_EXPR>: Suppress -Wreturn-type warning
> > if RETURN_EXPR has erroneous argument.
> > gcc/testsuite/
> > * g++.target/i386/pr118068.C: New test.
> >
> > --- gcc/cp/cp-gimplify.cc.jj 2025-03-06 17:26:00.547975990 +0100
> > +++ gcc/cp/cp-gimplify.cc 2025-03-07 12:07:41.578078222 +0100
> > @@ -519,7 +519,7 @@ cp_fold_immediate (tree *tp, mce_value m
> > cp_fold_data data (flags);
> > int save_errorcount = errorcount;
> > - tree r = cp_walk_tree_without_duplicates (tp, cp_fold_immediate_r,
> > &data);
> > + tree r = cp_walk_tree (tp, cp_fold_immediate_r, &data, NULL);
> > if (errorcount > save_errorcount)
> > return integer_one_node;
> > return r;
> > @@ -1204,13 +1204,14 @@ cp_build_init_expr_for_ctor (tree call,
> > return init;
> > }
> > -/* A subroutine of cp_fold_r to handle immediate functions. */
> > +/* A walk_tree callback for cp_fold_function and cp_fully_fold_init to
> > handle
> > + immediate functions. */
> > static tree
> > cp_fold_immediate_r (tree *stmt_p, int *walk_subtrees, void *data_)
> > {
> > auto data = static_cast<cp_fold_data *>(data_);
> > tree stmt = *stmt_p;
> > /* The purpose of this is not to emit errors for mce_unknown. */
> > const tsubst_flags_t complain = (data->flags & ff_mce_false
> > ? tf_error : tf_none);
> > @@ -1250,7 +1251,19 @@ cp_fold_immediate_r (tree *stmt_p, int *
> > if (!ADDR_EXPR_DENOTES_CALL_P (stmt))
> > decl = TREE_OPERAND (stmt, 0);
> > break;
> > + case IF_STMT:
> > + if (IF_STMT_CONSTEVAL_P (stmt))
> > + {
> > + if (!data->pset.add (stmt))
> > + cp_walk_tree (&ELSE_CLAUSE (stmt), cp_fold_immediate_r, data_,
> > + NULL);
> > + *walk_subtrees = 0;
> > + return NULL_TREE;
> > + }
> > + /* FALLTHRU */
> > default:
> > + if (data->pset.add (stmt))
> > + *walk_subtrees = 0;
> > return NULL_TREE;
> > }
> > @@ -1370,45 +1383,8 @@ cp_fold_r (tree *stmt_p, int *walk_subtr
> > tree stmt = *stmt_p;
> > enum tree_code code = TREE_CODE (stmt);
> > - if (cxx_dialect >= cxx20)
> > - {
> > - /* Unfortunately we must handle code like
> > - false ? bar () : 42
> > - where we have to check bar too. The cp_fold call below could
> > - fold the ?: into a constant before we've checked it. */
> > - if (code == COND_EXPR)
> > - {
> > - auto then_fn = cp_fold_r, else_fn = cp_fold_r;
> > - /* See if we can figure out if either of the branches is dead. If it
> > - is, we don't need to do everything that cp_fold_r does. */
> > - cp_walk_tree (&TREE_OPERAND (stmt, 0), cp_fold_r, data, nullptr);
> > - if (integer_zerop (TREE_OPERAND (stmt, 0)))
> > - then_fn = cp_fold_immediate_r;
> > - else if (integer_nonzerop (TREE_OPERAND (stmt, 0)))
> > - else_fn = cp_fold_immediate_r;
> > -
> > - if (TREE_OPERAND (stmt, 1))
> > - cp_walk_tree (&TREE_OPERAND (stmt, 1), then_fn, data,
> > - nullptr);
> > - if (TREE_OPERAND (stmt, 2))
> > - cp_walk_tree (&TREE_OPERAND (stmt, 2), else_fn, data,
> > - nullptr);
> > - *walk_subtrees = 0;
> > - /* Don't return yet, still need the cp_fold below. */
> > - }
> > - else
> > - cp_fold_immediate_r (stmt_p, walk_subtrees, data);
> > - }
>
> If we're removing the call to cp_fold_immediate_r, we should copy over the
> early "this affects cp_fold_r" exit from that function that was previously
> removed as redundant.
You mean
/* No need to look into types or unevaluated operands.
NB: This affects cp_fold_r as well. */
if (TYPE_P (stmt) || unevaluated_p (TREE_CODE (stmt)))
{
*walk_subtrees = 0;
return NULL_TREE;
}
? Note, GCC 13 didn't have anything like that in cp_fold_r.
> > @@ -1537,6 +1513,17 @@ cp_fold_function (tree fndecl)
> > been constant-evaluated already if possible, so we can safely
> > pass ff_mce_false. */
> > cp_fold_data data (ff_genericize | ff_mce_false);
> > + /* Do cp_fold_immediate_r in separate whole IL walk instead of during
> > + cp_fold_r, as otherwise expressions using results of immediate
> > functions
> > + might not be folded as cp_fold is called on those before cp_fold_r is
> > + called on their argument. And calling cp_fold_immediate_r during
> > + cp_fold can mean evaluation of the immediate functions many times. */
>
> Hmm, fold_cache should prevent multiple evaluation?
So would you prefer something like the patch below instead?
Currently it regresses
FAIL: g++.dg/cpp2a/consteval-prop15.C -std=gnu++20 (test for excess errors)
FAIL: g++.dg/cpp2a/consteval-prop15.C -std=gnu++23 (test for excess errors)
FAIL: g++.dg/cpp2a/consteval-prop15.C -std=gnu++26 (test for excess errors)
FAIL: g++.dg/cpp2a/consteval-prop2.C -std=c++20 (test for warnings, line 10)
FAIL: g++.dg/cpp2a/consteval-prop2.C -std=c++23 (test for warnings, line 10)
FAIL: g++.dg/cpp2a/consteval-prop2.C -std=c++26 (test for warnings, line 10)
and fixes the new testcase in the patch.
The excess errors are
/usr/src/gcc/gcc/testsuite/g++.dg/cpp2a/consteval-prop15.C:104:6: error: call
to consteval function 'f9<int>(non_const)' is not a constant expression
/usr/src/gcc/gcc/testsuite/g++.dg/cpp2a/consteval-prop15.C:104:6: error:
'non_const' is not a constant expression
/usr/src/gcc/gcc/testsuite/g++.dg/cpp2a/consteval-prop15.C:106:7: error: call
to consteval function 'f10<int>(non_const)' is not a constant expression
/usr/src/gcc/gcc/testsuite/g++.dg/cpp2a/consteval-prop15.C:106:7: error:
'non_const' is not a constant expression
And in the other tests the differences on stderr are
-consteval-prop2.C:10:16: note: ‘constexpr int f(T) [with T = int]’ was
promoted to an immediate function because its body contains an
immediate-escalating expression ‘id(t)’
- 10 | return t + id(t); // { dg-message "immediate-escalating
expression .id\\(t\\)." }
- | ~~^~~
+consteval-prop2.C:14:10: note: ‘constexpr int f(T) [with T = int]’ was
promoted to an immediate function
and
-consteval-prop2.C:71:21: error: ‘i’ is not a constant expression
+consteval-prop2.C:71:22: error: ‘i’ is not a constant expression
+ 71 | + k<int>(i); // { dg-bogus "call to|constant" "" { xfail
*-*-* } }
+ | ^
2025-03-17 Jakub Jelinek <ja...@redhat.com>
PR target/118068
gcc/cp/
* cp-gimplify.cc (get_fold_cache): Add prototype.
(cp_fold_immediate_r): For CALL_EXPR or ADDR_EXPR, query fold_cache
and if stmt is there already and not mapped to self, exit early.
(cp_genericize_r) <case RETURN_EXPR>: Suppress -Wreturn-type warning
if RETURN_EXPR has erroneous argument.
(cp_fold) <case ADDR_EXPR>: For C++20 call cp_fold_immediate_r.
(cp_fold) <case CALL_EXPR>: Likewise.
gcc/testsuite/
* g++.target/i386/pr118068.C: New test.
--- gcc/cp/cp-gimplify.cc.jj 2025-03-14 15:31:40.170197353 +0100
+++ gcc/cp/cp-gimplify.cc 2025-03-17 16:51:14.644040172 +0100
@@ -1204,6 +1204,8 @@ cp_build_init_expr_for_ctor (tree call,
return init;
}
+static hash_map<tree, tree> *&get_fold_cache (fold_flags_t);
+
/* A subroutine of cp_fold_r to handle immediate functions. */
static tree
@@ -1257,6 +1259,22 @@ cp_fold_immediate_r (tree *stmt_p, int *
if (!decl || TREE_CODE (decl) != FUNCTION_DECL)
return NULL_TREE;
+ /* If this call has been evaluated before by cp_fold and
+ stored into the fold cache, just use earlier computed replacement. */
+ if (code == CALL_EXPR || code == ADDR_EXPR)
+ {
+ auto &fold_cache = get_fold_cache (data->flags);
+ if (fold_cache != NULL)
+ {
+ if (tree *cached = fold_cache->get (stmt))
+ if (*cached != stmt)
+ {
+ *stmt_p = *cached;
+ return NULL_TREE;
+ }
+ }
+ }
+
/* Fully escalate once all templates have been instantiated. What we're
calling is not a consteval function but it may become one. This
requires recursing; DECL may be promoted to consteval because it
@@ -1717,6 +1735,11 @@ cp_genericize_r (tree *stmt_p, int *walk
case RETURN_EXPR:
if (TREE_OPERAND (stmt, 0))
{
+ if (error_operand_p (TREE_OPERAND (stmt, 0))
+ && warn_return_type)
+ /* Suppress -Wreturn-type for this function. */
+ suppress_warning (current_function_decl, OPT_Wreturn_type);
+
if (is_invisiref_parm (TREE_OPERAND (stmt, 0)))
/* Don't dereference an invisiref RESULT_DECL inside a
RETURN_EXPR. */
@@ -3082,6 +3105,19 @@ cp_fold (tree x, fold_flags_t flags)
goto unary;
case ADDR_EXPR:
+ if (cxx_dialect >= cxx20
+ && !ADDR_EXPR_DENOTES_CALL_P (x)
+ && TREE_CODE (TREE_OPERAND (x, 0)) == FUNCTION_DECL)
+ {
+ int walk_subtrees = 0;
+ cp_fold_data data (flags);
+ cp_fold_immediate_r (&x, &walk_subtrees, &data);
+ if (x != org_x)
+ {
+ x = cp_fold (x, flags);
+ break;
+ }
+ }
loc = EXPR_LOCATION (x);
op0 = cp_fold_maybe_rvalue (TREE_OPERAND (x, 0), false, flags);
@@ -3320,6 +3356,17 @@ cp_fold (tree x, fold_flags_t flags)
case CALL_EXPR:
{
+ if (cxx_dialect >= cxx20)
+ {
+ int walk_subtrees = 0;
+ cp_fold_data data (flags);
+ cp_fold_immediate_r (&x, &walk_subtrees, &data);
+ if (x != org_x)
+ {
+ x = cp_fold (x, flags);
+ break;
+ }
+ }
tree callee = get_callee_fndecl (x);
/* "Inline" calls to std::move/forward and other cast-like functions
--- gcc/testsuite/g++.target/i386/pr118068.C.jj 2025-03-07 10:38:15.551662990
+0100
+++ gcc/testsuite/g++.target/i386/pr118068.C 2025-03-07 10:38:15.551662990
+0100
@@ -0,0 +1,17 @@
+// PR target/118068
+// { dg-do compile { target c++20 } }
+// { dg-options "-O0 -mavx" }
+
+typedef float V __attribute__((vector_size (32)));
+
+consteval unsigned char
+foo (int x)
+{
+ return x;
+}
+
+V
+bar (V x, V y)
+{
+ return __builtin_ia32_blendps256 (x, y, (int) foo (0x23));
+}
Jakub
