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
Hello,
> Non-local goto's are handled simply by leaving them in the program as a
> GOTO_EXPR stmt, correct?
yes.
> > 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?
Not sure; I simply copied the code from the functions
cleanup_{switch,cond}_expr graph.
> > + /* 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...
Yes.
> > *************** 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?
No, we always keep at most one edge to the same block.
> > 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?
Good question :-) I am obviously accessing recently freed memory here,
which works by pure luck; I will fix this.
> And isn't moving delete_tree_ssa unrelated to this patch?
No -- delete_tree_ssa kills statement annotations, including bb_for_stmt
info that is needed for delete_tree_cfg to work.
> > 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.
No, see above.
> 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?
Of course, I will send it soon.
Zdenek