Bad code from egcs-1.1b

[Jason Merrill]

>>>>> Jeffrey A Law <law@cygnus.com> writes:

 >   In message < u9g1cbc2u2.fsf@yorick.cygnus.com >you write:
 >> >>>>> Jeffrey A Law <law@cygnus.com> writes:
 >> 
 >>  > jump.c::duplicate_loop_exit_test simply refuses to roll loops in this
 >>  > manner if it finds a BLOCK_{BEGIN,END} note.  We could do the same.  I
 >>  > don't see many alternatives.
 >> 
 >> The jump code refuses because duplicating the notes is invalid.  Moving
 >> them isn't, so long as the order is preserved, and it seems to me that the
 >> order would only be broken iff the test contains block notes and the loop
 >> body contains block notes.  That seems quite testable.

 > Well, if jump.c moved them, then we'd probably be in the same boat as we
 > are in stmt.c :-)

Oops, communications failure; I'm talking about moving the notes in
expand_end_loop.  We currently don't because we're afraid of breaking the
order, but in most cases moving them won't cause any problems.

Since our testcase uses do-while, the compiler treats the whole loop body
as part of the "test", leaving nothing in the "body".  In this case, it's
perfectly safe to move the notes because there are no block notes in the
"body".

The current code disables moving if it finds block notes in the test and
optimize < 2; one fix would be to always move block notes, but disable
reorganization if there are block notes in both test and body regardless of
the optimization level.  This might have a minor optimization impact, but I
suspect that code with blocks in both test and body is rare.  Or is it?
That might break reorganization of

  while (1)
    {
      if (foo)
        break;
    }

OK, never mind that idea.

 >> Another possibility would be to associate the fixup with something
 >> other than a block; perhaps a code_label?

 > This is interesting.  Assuming I understand the problem, you actually
 > don't want to associate with the note or any existing label, but instead
 > the edge as you leave an inner scope to an outer scope.

Yes.  Specifically, we want to associate with the jump that is emitted just
after we call expand_fixup.

 > We don't want to associate with the inner scope due to the problem we're
 > trying to solve now.

Not really; the problem we're trying to solve now seems to me like it's
just an implementation artifact.

 > We don't want to associate with anything that currently exists in the
 > outer scope because any point we select may potentially be reachable
 > from multiple threads of control.

Essentially.  fixup_gotos also says something about the things being
cleaned up going out of scope; that may be referring to the EH issue below.

 > Instead we want to run cleanups after we exit the inner scope, but
 > before we enter the outer scope.

Yes and no.  EH makes this more complicated; semantically, each cleanup is
in the next outer EH region.  We currently handle this by wrapping the
fixup cleanups in their own regions which just rethrow to the region
outside the one they are the cleanup for.  If we have popped the inner
scope, their regions will not be on ehstack anymore, so we won't be able to
find the right place to rethrow to.

 >>  > Is there any way to distinguish between a random BLOCK_{BEG,END} and
 >>  > one used for placement of cleanups?
 >> 
 >> You could scan goto_fixup_chain, I suppose.  See expand_fixup for how
 >> they are created.

 > Yea, that looks pretty straightforward.  I can do this if you don't want
 > to try and tackle the potential solution I mentioned above.

Couldn't we just use another kind of note?  Or, for that matter, keep a
pointer to the jump?

Jason