This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [tree-ssa] Splitting abnormal edges
- From: Andrew MacLeod <amacleod at redhat dot com>
- To: Jeff Sturm <jsturm at one-point dot com>
- Cc: gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: 16 Jun 2003 10:01:16 -0400
- Subject: Re: [tree-ssa] Splitting abnormal edges
- References: <Pine.LNX.4.44.0306160010420.692-100000@ops2.one-point.com>
On Mon, 2003-06-16 at 00:41, Jeff Sturm wrote:
> On 13 Jun 2003, Andrew MacLeod wrote:
> > * tree-cfg.c (bsi_commit_first_edge_insert): Only consider non-abnormal
> > edges when determining whether an edge needs to be split.
>
> I think I can explain why this didn't help my Java tests.
>
> We ignore abnormal outgoing edges for consideration in edge splitting,
> which is fine:
>
> > ! for (e2 = src->succ; e2; e2 = e2->succ_next)
> > ! if (!(e2->flags & EDGE_ABNORMAL))
> > ! num_exit++;
>
> though we later fall back on edge splitting for control-altering blocks:
>
> if (!is_ctrl_stmt (last) && !is_ctrl_altering_stmt (last))
> {
> bsi_insert_after (&bsi, stmt, BSI_NEW_STMT);
> return bsi;
> }
>
> The trouble being that is_ctrl_altering_stmt often returns 1 for
> statements we don't care about, i.e. certain CALL_EXPR and MODIFY_EXPR
> nodes that merely introduce an abnormal edge.
>
Yes, you are correct here. We only care about RETURN and GOTO out of
is_ctrl_altering_stmt() now.
> For instance, the block in question has:
>
> BLOCK 12
> PRED: 11
> SUCC: 16 13 (ab)
> PARENT: 7
> LOOP DEPTH: 0
> NEXT BLOCK: 13
> PREV BLOCK: 11
> 18 T.12 = T.11
> 18 parseInt.13 = (int (*<UPFNde00>) (struct java.lang.String *))parseInt
> 18 T.14 = parseInt.13 (T.12)
>
>
> The quick patch below fixes this case but isn't quite right, since it
> doesn't consider functions that longjmp or don't return.
>
We do we care about those? If they have one Normal edge going out,
inserting after the stmt would still be fine would it not?
> Perhaps it would be cleaner if is_ctrl_altering_stmt() were split into two
> functions, with something like stmt_may_throw_p(). Or have it
> consider EDGE_FALLTHRU cases only. Andrew, what do you think?
>
hmm. If we insert after the last stmt in the block, we end up ending the
basic block with a non-block ending stmt. Ie, the flow graph will not be
in an expected state... I guess what we really need is a new basic block
that the stmt falls through to,
THat shouldn't be too hard to do.
So the default case in find_insert_location should check for
is_ctrl_altering_stmt, and return src->end_tree_p,and return
EDGE_INSERT_LOCATION_AFTER;
LIke your original patch, excep check is_ctrl_altering_stmt() for true.
If it isn't true, we should abort(). I was going to object to your
earlier patch because I wanted to make the default abort(). Now we
should check this, then abort. IS there a reason this doesn't work?
Try something like this. (DO we need any cases for CATCH, TRY or
anything else?)