This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [tree-ssa] Removal of gotos from cfg based ir
On Mon, 2003-11-17 at 20:24, Zdenek Dvorak wrote:
Non-local goto's are handled simply by leaving them in the program as a
GOTO_EXPR stmt, correct?
> bool
> ! cleanup_control_expr_graph (basic_block bb, block_stmt_iterator bsi)
> {
> edge taken_edge;
> bool retval = false;
> ! tree expr = bsi_stmt (bsi), val;
>
> if (bb->succ->succ_next)
> {
> edge e, next;
>
> ! switch (TREE_CODE (expr))
> ! {
> ! case COND_EXPR:
> ! val = COND_EXPR_COND (expr);
> ! break;
> !
> ! case SWITCH_EXPR:
> ! val = SWITCH_COND (expr);
> ! if (TREE_CODE (val) != INTEGER_CST)
> ! return false;
> ! break;
> !
> ! default:
> ! abort ();
> ! }
> !
> taken_edge = find_taken_edge (bb, val);
So we can't find_taken_edge for switches with non-integer values, but we
can for COND_EXPR?
> + /* Add gotos that used to be represented implicitly in cfg. */
> +
> + static void
> + disband_implicit_edges (void)
> + {
> + basic_block bb;
> + block_stmt_iterator last;
> + edge e;
> +
> + FOR_EACH_BB (bb)
> + {
> + last = bsi_last (bb);
> +
> + /* Find a fallthru edge and emit the goto if neccesary. */
> + for (e = bb->succ; e; e = e->succ_next)
> + if (e->flags & EDGE_FALLTHRU)
> + break;
Does the cfg verifier ensure there is never more than 1 fallthru edge? I
assume so...
> *************** tree_try_redirect_by_replacing_jump (edg
> *** 3381,3402 ****
> stmt = bsi_stmt (b);
>
> if (TREE_CODE (stmt) == COND_EXPR
> ! || TREE_CODE (stmt) == SWITCH_EXPR
> ! || (TREE_CODE (stmt) == GOTO_EXPR && target == src->next_bb))
> {
> ! if (target == src->next_bb)
> ! {
> ! flags = EDGE_FALLTHRU;
> ! bsi_remove (&b);
> ! }
> ! else
> ! {
> ! flags = 0;
> ! stmt = build1 (GOTO_EXPR, void_type_node, tree_block_label (target));
> ! bsi_replace (&b, stmt);
> ! }
> e = ssa_redirect_edge (e, target);
> ! e->flags = flags;
> return e;
> }
>
> --- 3264,3274 ----
> stmt = bsi_stmt (b);
>
> if (TREE_CODE (stmt) == COND_EXPR
> ! || TREE_CODE (stmt) == SWITCH_EXPR)
> {
> ! bsi_remove (&b);
> e = ssa_redirect_edge (e, target);
> ! e->flags = EDGE_FALLTHRU;
> return e;
> }
>
Out of curiosity, shouldn't any other edges be removed here? We check at
the top of the routine that they all go to the same target, but can't we
end up with 2 (or more) edges to the same block? Or does someone else
clean that up?
> Index: tree-optimize.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/tree-optimize.c,v
> retrieving revision 1.1.4.73
> diff -c -3 -p -r1.1.4.73 tree-optimize.c
> *** tree-optimize.c 17 Nov 2003 23:39:20 -0000 1.1.4.73
> --- tree-optimize.c 18 Nov 2003 01:14:26 -0000
> *************** optimize_function_tree (tree fndecl, tre
> *** 176,181 ****
> --- 176,184 ----
> sbitmap_free (vars_to_rename);
> }
>
> + delete_tree_cfg ();
> + delete_tree_ssa (fndecl);
> +
> /* Re-chain the statements from the blocks. */
> {
> basic_block bb;
> *************** optimize_function_tree (tree fndecl, tre
> *** 184,191 ****
> FOR_EACH_BB (bb)
> append_to_statement_list_force (bb->stmt_list, chain);
> }
> -
> - delete_tree_cfg ();
> }
>
Aren't you deleting the CFG, then looping over the BB's to rechain the
blocks? How can that work?
And isn't moving delete_tree_ssa unrelated to this patch?
> --- 187,192 ----
> Index: tree-pretty-print.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/Attic/tree-pretty-print.c,v
> retrieving revision 1.1.2.61
> diff -c -3 -p -r1.1.2.61 tree-pretty-print.c
> *** tree-pretty-print.c 17 Nov 2003 12:08:26 -0000 1.1.2.61
> --- tree-pretty-print.c 18 Nov 2003 01:14:26 -0000
> *************** dump_generic_node (pretty_printer *buffe
> *** 734,740 ****
> else
> first = false;
> dump_generic_node (buffer, tsi_stmt (si), spc, flags, true);
> - pp_character (buffer, ';');
> }
> }
> break;
> --- 734,739 ----
> *************** dump_generic_node (pretty_printer *buffe
> *** 1282,1287 ****
> --- 1281,1287 ----
> dump_generic_node (buffer, elt, spc+4, flags, false);
> pp_string (buffer, " goto ");
> dump_generic_node (buffer, CASE_LABEL (elt), spc+4, flags, true);
> + pp_semicolon (buffer);
> }
> }
> newline_and_indent (buffer, spc+2);
This is also unrelated to this patch.
> *************** dump_phi_nodes (pretty_printer *buffer,
> *** 2150,2155 ****
> --- 2150,2187 ----
> }
> }
>
> + /* Dump jump to basic block BB that is represented implicitly in the cfg
> + to BUFFER. */
> +
> + static void
> + pp_cfg_jump (pretty_printer *buffer, basic_block bb)
> + {
> + pp_string (buffer, "goto <bb ");
> + pp_decimal_int (buffer, bb->index);
> + pp_string (buffer, ">;");
> + }
> +
> + /* Dump edges represented implicitly in basic block BB to BUFFER, indented
> + by INDENT spaces, with details given by FLAGS. */
> +
> + static void
> + dump_implicit_edges (pretty_printer *buffer, basic_block bb, int indent)
> + {
> + edge e;
> +
> + /* If there is a fallthru edge, we may need to add an artificial goto to the
> + dump. */
> + for (e = bb->succ; e; e = e->succ_next)
> + if (e->flags & EDGE_FALLTHRU)
> + break;
> + if (e && e->dest != bb->next_bb)
> + {
> + INDENT (indent);
> + pp_cfg_jump (buffer, e->dest);
> + pp_newline (buffer);
> + }
> + }
> +
I like this way of showing the implicit GOTOs.
> Index: tree-ssa.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/Attic/tree-ssa.c,v
> retrieving revision 1.1.4.154
> diff -c -3 -p -r1.1.4.154 tree-ssa.c
> *** tree-ssa.c 18 Nov 2003 00:17:51 -0000 1.1.4.154
> --- tree-ssa.c 18 Nov 2003 01:14:26 -0000
> *************** static void rewrite_initialize_block (st
> *** 177,183 ****
> static void rewrite_walk_stmts (struct dom_walk_data *, basic_block, tree);
> static void rewrite_add_phi_arguments (struct dom_walk_data *,
> basic_block, tree);
> - static void delete_tree_ssa (tree);
> static void mark_def_sites (struct dom_walk_data *walk_data,
> basic_block bb,
> tree parent_block_last_stmt ATTRIBUTE_UNUSED);
> --- 177,182 ----
> *************** rewrite_out_of_ssa (tree fndecl, enum tr
> *** 2494,2500 ****
> }
>
> /* Flush out flow graph and SSA data. */
> - delete_tree_ssa (fndecl);
> delete_var_map (map);
> timevar_pop (TV_TREE_SSA_TO_NORMAL);
> }
<...>
All the changes to tree-ssa.c appear to be unrelated to this patch. Are
they? if so, please submit any unrelated items seperately.
Other than that, the patch looks reasonable to me. It of course doesn't
apply to the branch any more (:-|). can you submit a new one that
applies with any relevent changes above for a final look?
Thanks
Andrew