This is the mail archive of the 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: [patch] for PR 14229

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

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,

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