On Tue, Mar 18, 2025 at 12:26:53PM -0400, Jason Merrill wrote:
> > 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.
>
> Hmm, indeed, I was misremembering. I still think it would be good to add
> it.
Ok. Shall I do it in this patch or incrementally?
All of
/* No need to look into types or unevaluated operands.
NB: This affects cp_fold_r as well. */
if (TYPE_P (stmt)
|| unevaluated_p (code)
/* We do not use in_immediate_context here because it checks
more than is desirable, e.g., sk_template_parms. */
|| cp_unevaluated_operand
|| (current_function_decl
&& DECL_IMMEDIATE_FUNCTION_P (current_function_decl)))
{
*walk_subtrees = 0;
return NULL_TREE;
}
and unconditionally or just for C++ >= 20?
The current behavior before the patch is that because cp_fold_immediate_r
does that, with the exception of COND_EXPRE we do essentially
if (cxx_dialect >= cxx20
&& (TYPE_P (stmt)
|| unevaluated_p (code)
/* We do not use in_immediate_context here because it checks
more than is desirable, e.g., sk_template_parms. */
|| cp_unevaluated_operand
|| (current_function_decl
&& DECL_IMMEDIATE_FUNCTION_P (current_function_decl))))
*walk_subtrees = 0;
(so we clear *walk_subtrees but still continue to call cp_fold on it
even in those cases, and do it only for C++20).
> > > > @@ -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?
>
> Hmm, no, I think your earlier approach is cleaner. I'd just like to remove
> the last sentence of the comment.
Patch with just that removed sentence is below.
2025-03-18 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);
- }
-
*stmt_p = stmt = cp_fold (*stmt_p, data->flags);
- /* For certain trees, like +foo(), the cp_fold above will remove the +,
- and the subsequent tree walk would go straight down to the CALL_EXPR's
- operands, meaning that cp_fold_immediate_r would never see the
- CALL_EXPR. Ew :(. */
- if (TREE_CODE (stmt) == CALL_EXPR && code != CALL_EXPR)
- cp_fold_immediate_r (stmt_p, walk_subtrees, data);
-
if (data->pset.add (stmt))
{
/* Don't walk subtrees of stmts we've already walked once, otherwise
@@ -1537,6 +1513,16 @@ 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. */
+ if (cxx_dialect >= cxx20)
+ {
+ cp_walk_tree (&DECL_SAVED_TREE (fndecl), cp_fold_immediate_r,
+ &data, NULL);
+ data.pset.empty ();
+ }
cp_walk_tree (&DECL_SAVED_TREE (fndecl), cp_fold_r, &data, NULL);
/* This is merely an optimization: if FNDECL has no i-e expressions,
@@ -1717,6 +1703,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. */
@@ -2910,6 +2901,11 @@ cp_fully_fold_init (tree x)
return x;
x = cp_fully_fold (x, mce_false);
cp_fold_data data (ff_mce_false);
+ if (cxx_dialect >= cxx20)
+ {
+ cp_walk_tree (&x, cp_fold_immediate_r, &data, NULL);
+ data.pset.empty ();
+ }
cp_walk_tree (&x, cp_fold_r, &data, NULL);
return x;
}
--- 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
