This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [tree-ssa] Splitting abnormal edges


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?)



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]