On Thu, Nov 26, 2015 at 12:34:55PM +0000, Joseph Myers wrote: > On Thu, 26 Nov 2015, Marek Polacek wrote: > > > I had a go at this, but I'm now skeptical about removing c_save_expr. > > save_expr calls fold (), so we need to ensure that we don't pass any > > C_MAYBE_CONST_EXPRs into it, meaning that we'd need to call c_fully_fold > > before > > save_expr anyway... > > > > So maybe go the "remove C_MAYBE_CONST_EXPRs in SAVE_EXPRs in > > c_gimplify_expr" > > way? > > I believe it should be safe for gimplification to process > C_MAYBE_CONST_EXPR in the same way c_fully_fold_internal does. That is, > this should not affect correctness. If a C_MAYBE_CONST_EXPR got through > to gimplification, in some cases it may mean that something did not get > properly folded with c_fully_fold as it should have done - but if the move > to match.pd means all optimizations currently done with fold end up > working on GIMPLE as well, any missed optimizations from this should > disappear (and if we can solve the diagnostics issues, eventually fewer > calls to c_fully_fold should be needed and they should be more about > checking what can occur in constant expressions and less about folding for > optimization). So here's an attempt to strip C_MAYBE_CONST_EXPRs, only for SAVE_EXPRs, because c_fully_fold in c_process_stmt_expr should deal with other expressions.
My worry was of course C_MAYBE_CONST_EXPR_PRE. But it seems we'll never have any at that point, since it's already been processed and transformed to a COMPOUND_EXPR. But do I like this patch? No. > The general principle of delaying folding also means that we should move > away from convert_* folding things. Yep, I tried using _nofold variants, but it had soem fallout. Anyway, something for next stage1. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2015-11-26 Marek Polacek <pola...@redhat.com> PR c/68513 * c-gimplify.c (strip_c_maybe_const_expr_r): New. (c_gimplify_expr): Call it. * gcc.dg/torture/pr68513.c: New test. diff --git gcc/c-family/c-gimplify.c gcc/c-family/c-gimplify.c index fc4a44a..c096575 100644 --- gcc/c-family/c-gimplify.c +++ gcc/c-family/c-gimplify.c @@ -212,6 +212,21 @@ c_build_bind_expr (location_t loc, tree block, tree body) /* Gimplification of expression trees. */ +/* Callback for c_gimplify_expr. Strip C_MAYBE_CONST_EXPRs in TP so that + they don't leak into the middle end. */ + +static tree +strip_c_maybe_const_expr_r (tree *tp, int *walk_subtrees, void *) +{ + if (TREE_CODE (*tp) == C_MAYBE_CONST_EXPR) + { + gcc_assert (C_MAYBE_CONST_EXPR_PRE (*tp) == NULL_TREE); + *tp = C_MAYBE_CONST_EXPR_EXPR (*tp); + *walk_subtrees = 0; + } + return NULL_TREE; +} + /* Do C-specific gimplification on *EXPR_P. PRE_P and POST_P are as in gimplify_expr. */ @@ -296,6 +311,10 @@ c_gimplify_expr (tree *expr_p, gimple_seq *pre_p ATTRIBUTE_UNUSED, return (enum gimplify_status) gimplify_cilk_spawn (expr_p); } + case SAVE_EXPR: + walk_tree_without_duplicates (expr_p, strip_c_maybe_const_expr_r, NULL); + break; + default:; } diff --git gcc/testsuite/gcc.dg/torture/pr68513.c gcc/testsuite/gcc.dg/torture/pr68513.c index e69de29..4e08b29 100644 --- gcc/testsuite/gcc.dg/torture/pr68513.c +++ gcc/testsuite/gcc.dg/torture/pr68513.c @@ -0,0 +1,13 @@ +/* PR c/68513 */ +/* { dg-do compile } */ + +int i; +unsigned u; +volatile unsigned int *e; + +void +fn1 (void) +{ + (short) ((i ? *e : 0) & ~u | i & u); + (short) (((0, 0) ? *e : 0) & ~u | i & u); +} Marek