Re: [patch] for PR 14229

[Jim Wilson <wilson at specifixinc dot com>]

On Thu, 2004-02-26 at 02:12, Zdenek Dvorak wrote:
> the last statement.  Tidy_fallthru_edge is a no-op in this case, which
> is OK on other places where it is used, but not here.

So the problem is that try_simplify_condjump assumes that
tidy_fallthru_edge will clean up after us, but in the case of a
!only_jump_p it does not?  The only thing that tidy_fallthru edge might
do is set an edge to fallthough.  It isn't clear why this matters.

In fact, looking at rtl_tidy_fallthru_edge, I see that it is already
confused. There is an !onlyjump_p test at the top, and it returns if
true.  Then in the middle, it tests onlyjump_p as part of an if, but
this test is redundant.  It will always succeed, because of the earlier
test.  Since rtl_tidy_fallthru_edge is already broken, maybe the bug is
there.

I was really hoping for an analysis and explanation of the patch here. 
I believe that we need to understand what the code does before we can
fix it.  A patch that appears to give correct results is not always a
correct patch.  I did not get an explanation I could understand here.

I debugged the problem myself so I could see what was going on.  The
problem seems to be that the delete_basic_block call deletes the
following block and the edge that leads to it.  That leaves us with a
conditional branch with only one outgoing edge, which is not marked as a
fall-through edge.  Other code interprets this as an error unless there
is a barrier present, and there is no barrier in this case, there should
not be one.

So we do need rtl_tidy_fallthru_edge to mark the edge as a fallthru
edge, or else we need to disable the entire optimization at the top of
try_simplify_condjump which is what your patch does.

It isn't clear why rtl_tidy_fallthru_edge ignores !onlyjump_p branches. 
One could perhaps fix them by deleting the branch and re-emiting the
rest of the instructions in the parallel.  The current code which leaves
them unoptimized seems to unfairly penalize targets with
decrement_and_branch patterns like HPPA.  But this is an optimization
issue, and it is reasonable to handle this separately from the
correctness issue.

The patch is approved.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com