PATCH Re: [ast-optimizer-branch]: Simplify STMT_EXPR's
Daniel Berlin
dberlin@dberlin.org
Wed Jun 19 06:49:00 GMT 2002
On Wednesday, June 19, 2002, at 05:22 AM, Jason Merrill wrote:
> Here's a new patch for this. Some notes:
>
> Daniel's earlier patch would find the last EXPR_STMT at arbitrary depth.
> I don't see anything in the docs or existing practice to suggest that
> the return value can be nested further under the initial block, so I only
> look there.
I originally did this as well, relying on what the docs said. But IIRC, simplification accidently can create cases where the return value is nested under the initial block, and we need to handle this properly. It might be different now, remember that simplification has completely changed in the past few weeks. But, something like (This probably isn't a real example, but it should give you the idea):
({
int i;
for (i = 0; i < 50; i++)
{
<whatever>
}
i;
})
got changed into
({
int i;
{
i = 0;
for (; i < 50; )
{
<whatever>
i = i + 1;
}
i;
}
})
when we simplified the body.
In fact, if you look at your code below, you changed it so it simplifies the body *after* creating the modify expression for the return value.
I'm not sure this doesn't screw up exactly because of the above.
Have you checked to make sure the return value stays at the same scope level after we simplify the body?
We should probably add an abort if the last statement before the closing scope of the statement expression is now a scope end, rather than an executable statement (IE you have a close scope followed by a close scope, rather than an executable statement followed by a close scope), if it returns a value.
>
> In this patch I haven't tried to preserve source position information for
> inlined functions, because we don't support wrapping _STMTs in
> EXPR_WITH_FILE_LOCATION. One obvious direction would be to support that.
> I don't see a good way to use FILE_STMT to handle this, as we aren't
> tracking source position during simplification. On the other hand, perhaps
> we should be.
>
> I ran into trouble with bogus TREE_ANN information on statements in inlined
> functions; the annotation info is allocated using the GC code, but is never
> marked, which means that after the inline function has been compiled, all
> the TREE_ANN fields in its tree structure are pointing to freed memory.
> I've worked around this by clearing the aux field in copy_tree_r; a better
> fix would be to mark it properly for GC.
>
> Tested i686-pc-linux-gnu.
>
> 2002-06-19 Jason Merrill <jason@redhat.com>
>
> * c-simplify.c (simplify_stmt_expr): New fn.
> (simplify_expr): Call it.
> (stmt_expr_level): Remove.
> (stmt_has_effect, c_simplify_function_tree): Remove refs.
> (expr_has_effect): Deal with null expression.
> (simplify_expr_wfl): If the subexpression is simplified away, drop
> this one, too.
> * tree-simple.c (is_simple_unary_expr): Don't allow a STMT_EXPR.
> * tree-inline.c (copy_tree_r): Clear the aux field in the copies.
>
> *** c-simplify.c.~1~ Wed Jun 19 10:03:17 2002
> --- c-simplify.c Wed Jun 19 10:00:29 2002
> *************** static void simplify_compound_expr PAR
> *** 88,93 ****
> --- 88,94 ----
> static void simplify_expr_wfl PARAMS ((tree *, tree *, tree *,
> int (*) PARAMS ((tree)), tree));
> static void simplify_save_expr PARAMS ((tree *, tree *, tree));
> + static void simplify_stmt_expr PARAMS ((tree *, tree *));
> static void make_type_writable PARAMS ((tree));
> static tree add_tree PARAMS ((tree, tree *));
> static tree insert_before_continue PARAMS ((tree, tree));
> *************** static int is_last_stmt_of_scope PAR
> *** 105,117 ****
> static FILE *dump_file;
> static int dump_flags;
>
> - /* Used to keep track of statement expressions. Incremented each time we
> - start processing a statement expression. When simplifying statement
> - expressions, we need to keep some statements with no effect because they
> - might represent the return value of the statement expression. */
> - static int stmt_expr_level;
> -
> -
> /* Simplification of statement trees. */
>
> /** Entry point to the simplification pass. FNDECL is the FUNCTION_DECL
> --- 106,111 ----
> *************** c_simplify_function_tree (fndecl)
> *** 149,155 ****
> pushlevel (0);
>
> /* Simplify the function's body. */
> - stmt_expr_level = 0;
> simplify_stmt (&fnbody);
>
> /* Declare the new temporary variables. */
> --- 143,148 ----
> *************** simplify_expr (expr_p, pre_p, post_p, si
> *** 1122,1130 ****
> /* The following are special cases that are not handled by the original
> SIMPLE grammar. */
> case STMT_EXPR:
> ! stmt_expr_level++;
> ! simplify_stmt (&STMT_EXPR_STMT (*expr_p));
> ! stmt_expr_level--;
> break;
>
> /* SAVE_EXPR nodes are converted into a SIMPLE identifier and
> --- 1115,1121 ----
> /* The following are special cases that are not handled by the original
> SIMPLE grammar. */
> case STMT_EXPR:
> ! simplify_stmt_expr (expr_p, pre_p);
> break;
>
> /* SAVE_EXPR nodes are converted into a SIMPLE identifier and
> *************** simplify_expr_wfl (expr_p, pre_p, post_p
> *** 1865,1874 ****
> col = EXPR_WFL_COLNO (*expr_p);
>
> for (op = *pre_p; op; op = TREE_CHAIN (op))
> ! TREE_VALUE (op) = build_expr_wfl (TREE_VALUE (op), file, line, col);
>
> for (op = *post_p; op; op = TREE_CHAIN (op))
> TREE_VALUE (op) = build_expr_wfl (TREE_VALUE (op), file, line, col);
> }
>
> /** Simplify a SAVE_EXPR node. EXPR_P points to the expression to
> --- 1856,1870 ----
> col = EXPR_WFL_COLNO (*expr_p);
>
> for (op = *pre_p; op; op = TREE_CHAIN (op))
> ! /* FIXME! deal with inline source position info. */
> ! if (!statement_code_p (TREE_CODE (TREE_VALUE (op))))
> ! TREE_VALUE (op) = build_expr_wfl (TREE_VALUE (op), file, line, col);
>
> for (op = *post_p; op; op = TREE_CHAIN (op))
> TREE_VALUE (op) = build_expr_wfl (TREE_VALUE (op), file, line, col);
> +
> + if (EXPR_WFL_NODE (*expr_p) == NULL_TREE)
> + *expr_p = NULL_TREE;
> }
>
> /** Simplify a SAVE_EXPR node. EXPR_P points to the expression to
> *************** simplify_save_expr (expr_p, pre_p, stmt)
> *** 1903,1908 ****
> --- 1899,1943 ----
> }
> }
>
> + /* Simplify a STMT_EXPR. EXPR_P points to the expression to simplify.
> + After simplification, if the STMT_EXPR returns a value, EXPR_P will
> + point to a new temporary that holds that value; otherwise it will be
> + null.
> +
> + PRE_P points to the list where side effects that must happen before
> + *EXPR_P should be stored. */
> +
> + static void
> + simplify_stmt_expr (expr_p, pre_p)
> + tree *expr_p;
> + tree *pre_p;
> + {
> + tree body = STMT_EXPR_STMT (*expr_p);
> +
> + if (VOID_TYPE_P (TREE_TYPE (*expr_p)))
> + *expr_p = NULL_TREE;
> + else
> + {
> + tree substmt, last_expr_stmt, last_expr, temp, mod;
> +
> + last_expr_stmt = NULL_TREE;
> + for (substmt = COMPOUND_BODY (body); substmt;
> + substmt = TREE_CHAIN (substmt))
> + {
> + if (TREE_CODE (substmt) == EXPR_STMT)
> + last_expr_stmt = substmt;
> + }
> +
> + last_expr = EXPR_STMT_EXPR (last_expr_stmt);
> + temp = create_tmp_var (TREE_TYPE (last_expr), "retval");
> + mod = build_modify_expr (temp, NOP_EXPR, last_expr);
> + EXPR_STMT_EXPR (last_expr_stmt) = mod;
> + *expr_p = temp;
> + }
> +
> + simplify_stmt (&body);
> + add_tree (body, pre_p);
> + }
>
> /* Code generation. */
>
> *************** stmt_has_effect (stmt)
> *** 2499,2513 ****
> return 1;
> else if (expr_has_effect (EXPR_STMT_EXPR (stmt)))
> return 1;
> - else
> - {
> - /* The statement has no effect. However, if we are simplifying a
> - statement expression '({ ... })' and this statement may be the
> - last statement in the statement expression body, then it may
> - represent the return value of the statement expression. */
> - if (stmt_expr_level > 0 && is_last_stmt_of_scope (stmt))
> - return 1;
> - }
>
> return 0;
> }
> --- 2534,2539 ----
> *************** static int
> *** 2520,2525 ****
> --- 2546,2553 ----
> expr_has_effect (expr)
> tree expr;
> {
> + if (expr == NULL_TREE)
> + return 0;
> return (TREE_SIDE_EFFECTS (expr)
> || (TREE_CODE (expr) == CONVERT_EXPR
> && VOID_TYPE_P (TREE_TYPE (expr))));
> *** tree-simple.c.~1~ Wed Jun 19 10:03:17 2002
> --- tree-simple.c Tue Jun 18 18:26:43 2002
> *************** is_simple_unary_expr (t)
> *** 565,576 ****
> if (is_simple_cast (t))
> return 1;
>
> - /* Additions to the original grammar. FIXME: Statement-expressions should
> - really be expanded. */
> - if (TREE_CODE (t) == STMT_EXPR
> - && is_simple_stmt (STMT_EXPR_STMT (t)))
> - return 1;
> -
> /* Addition to the original grammar. Allow BIT_FIELD_REF nodes where
> operand 0 is a SIMPLE identifier and operands 1 and 2 are SIMPLE
> values. */
> --- 565,570 ----
> *** tree-inline.c.~1~ Wed Jun 19 10:03:17 2002
> --- tree-inline.c Wed Jun 19 10:07:05 2002
> *************** copy_tree_r (tp, walk_subtrees, data)
> *** 1381,1386 ****
> --- 1381,1389 ----
> /* Copy the node. */
> *tp = copy_node (*tp);
>
> + /* FIXME aux isn't marked properly for GC, so don't copy it. */
> + (*tp)->common.aux = 0;
> +
> /* Now, restore the chain, if appropriate. That will cause
> walk_tree to walk into the chain as well. */
> if (code == PARM_DECL || code == TREE_LIST
More information about the Gcc-patches
mailing list