Re: [patch] for PR 14229

[Jim Wilson <wilson at specifixinc dot com>]

On Fri, 2004-02-27 at 03:28, Zdenek Dvorak wrote:
> -   /* If the jump insn has side effects, we can't tidy the edge.  */
> -   if (GET_CODE (BB_END (b)) == JUMP_INSN
> -       && !onlyjump_p (BB_END (b)))
> -     return;
> - 

It isn't safe to remove this code if we don't know why it is there.  We
could remove it and then wait to see what breaks, and then figure out
what to do next.

Or we could add comments to your previous patch to
try_simplify_condjump.  We could note there that we can't handle
!onlyjump_p because tidy_fallthru_edge won't set the FALLTHRU bit for
us.  And then if someone wants this optimization, they can try to fix it
later.

Looking at the cvs history, I see that the !onlyjump_p test has been
there every since revision 1.1 of the cfgrtl.c file.  It was originally
in the tidy_fallthru_edges function.  Your big 2004-01-29 patch moves
the test from tidy_fallthru_edges to rtl_tidy_fallthu_edge, and that
caused try_simplify_condump to fail.  I don't see why it is necessary,
but there may be a reason that has been lost.  Maybe it was added before
tidy_fallthru_edge was fixed to avoid deleting !onlyjump_p branches?  In
which case the test would have been obsolete then, but we did not notice
it.

At this point, I think either patch is reasonable.  I will approve both
of them, and you can decide which way you want to go on this.  I'd lean
a bit towards the rtl_tidy_fallthru_edge change, because it removes a
redundant test, and allows the optimization to mostly succeed (except
for the part about deleting the branch).
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com