Re: ifcvt.c question

[Roger Sayle <roger at eyesopen dot com>]

On Sun, 29 May 2005, Steven Bosscher wrote:
> I don't understand what lines 2728 to 2741 are for.  Could someone give
> an example of when we can have a then_bb that has no successors, but
> still ends in something that satisfies simplejump_p()?  What kind of
> strange indirect jump would that be?  And shouldn't the check just use
> computed_jump_p() if that is what it is looking for??

For the benefit of the list, I did some archeology to determine when
this functionality was written and how it evolved to be so broken now.

The mistake was a bug introduced by Michael Meisner back in 2000.
The functionality was originally introduced by Richard Henderson to
allow predication of then blocks that end in noreturn calls.

2000-05-31  Richard Henderson  <rth@cygnus.com>

        * ifcvt.c (merge_if_block): Be prepared for JOIN to have no
        remaining edges.
        (find_if_block): Allow THEN with no outgoing edges.
        * flow.c (merge_blocks_nomove): Remove a barrier not following
        a jump as well.

posted: http://gcc.gnu.org/ml/gcc-patches/2000-05/msg01711.html


Unfortunately, this exposed a bug in the d30v port that caused a
regression in gcc.c-torture/execute/920428-1.c.  As I suggested on
IRC, this was a then-block that ended in a non-local goto, or more
precisely an indirect jump.

Michael tried to resolve this problem by excluding those blocks that
ended in indirect jumps.  His mistake was to assume that the way to
eliminate indirect jumps was to limit this code to simple jumps!
Doh!  This inadvertantly disabled RTH's optimization, and the
optimizations' code has been dead ever since.

2000-08-19  Michael Meissner  <meissner@redhat.com>

        * ifcvt.c (find_if_block): Do not assume that a THEN block has any
        instructions in it before checking for indirect jumps.

        * ifcvt.c (find_if_block): Do not consider a THEN block that ends
        in an indirect jump as a potential for conditional execution.

        * d30v.h (d30v_init_expanders): Don't declare here.
        * d30v-protos.h (d30v_init_expanders): Declare here with a valid
        prototype.

posted: http://gcc.gnu.org/ml/gcc-patches/2000-08/msg00736.html


RTH should probably have added a testcase that checked whether this
code was being optimized, so we'd know when it regressed.  I also can't
immediately see why a block that ends in an indirect jump shouldn't
be considered for conditional execution, and he might well have been
papering over another problem in the compiler (esp. the D30V backend).

Of course now that support for D30V has been removed from CVS, it's
difficult to diagnose whether this really was just a backend problem,
and whether we should just return the code to a form similar to the
one RTH originally contributed.

I hope this answers your question.

Roger
--