Re: Improvement to loop optimization in presence of inlining

[Jeffrey A Law <law at cygnus dot com>]

  In message <199807022037.NAA02152@smtp.earthlink.net>you write:
  > Right.  My original patch, though, would have scrogged it for the
  > whole file; this version only scrogs debugging the exit test for the
  > loop in question.
OK.  That's what I thought.  I just wanted to make sure I understood the
implications of the change :-)

  >     Jeffrey> stream if optimize > 1.
  > 
  > Sure; I'm happy to make that change.
I think this is wise.  Let's do it.

  > 
  >     Jeffrey> The other question is could the loop rotation create a
  >     Jeffrey> situation where the compiler would later delete those
  >     Jeffrey> notes?  And if so, is that safe considering the
  >     Jeffrey> relationship that has to be kept between the rtl and tree
  >     Jeffrey> structures for block scope info?
  > 
  > The compiler must never, ever delete NOTE_INSN_BLOCK_{BEG,END} notes,
  > and I don't think it does at present.  (Actually, I think this is a
  > misfeature; generated assembly tends to look like this in the presence
  > of lots of inlines:
The trick was I couldn't find code which would prevent us from removing
these notes.  Looking more closely at delete_insn I think we're OK in
this regard.  So consider this objection withdrawn.


  > i.e., lots of labels with nothing in them, and then debugging
  > information about the variables used in those blocks, even though
  > no-one will ever actually be in the block!  This wastes space in the
  > objects, and makes the assembly harder to read.  I've been planning to
  > fix this, but later; it's just annoying, not wrong.)
  > 
Right.  Agreed that we can look at it later.

  > OK, there are two issues here:
  > 
  >   o Was the code already broken?
I believe so, but that's just a general feeling based on a general
knowledge about some code reordering optimizations often being unsafe
in the presense of EH constructs.  I don't know a specific case that
fails, but my gut tells me we need to think about this issue and deal
with it.

  >   o Does my patch break it further?
I think so -- within the same constraints mentioned above -- my gut is
telling me there's issues (or maybe better stated questions) that need
to be addressed in this code.

  > Also, we should look at both changes independently.  First, let's
  > consider expand_end_loop:
Right.


  >          goto start_label;
  >          newstart_label:
  > 	 body;
  > 	 start_label:
  > 	 if (test) goto end_label;
  > 	 goto newstart_label;
  > 	 end_label;
  > 
  > If `test' contains partial EH-regions, there's a problem.  My patch
  > doesn't check for this, but neither did the earlier code.  
  > I agree that we should check this.  In other words, if we have:
Agreed on both counts -- I think the old code may be broken and you've
inherited that brokennsess.

  > This isn't legal C++, but here's what it's supposed to mean: if cond()
  > is true, stop looping.  Otherwise, call bar, and keep looping.  In
  > addition, if cond throws an exception, catch it and keep looping. Such
  > constructs are certainy legal in LISP.
OK -- so can it happen as a result of other optimizations?  Probably not
unless they're not EH aware too.  I wonder if Ada allows this kind of 
construct.

  > We should not move the test since then the EH-region for the try-block
  > would be broken up.  (In this case we would the EH_BEG note for the
  > `try' and `if cond()' but not the call to bar() or the EH_END note.)
Right.  I think we should at least detect this and avoid rotating the loop
just to be safe.

  > Now, on to jump.c.  I think there should be a 1:1 correspondence
  > between EH_REGIONs and catch handlers, so no-one should ever duplicate
  > an EH_REGION.
Hmm, is this true after cross-jumping?  Then again, cross jumping happens
so late in the compiler that it shouldn't be an issue.


  >   o Adjust my change to stmt.c so as to not copy partial EH-regions in
  >     stmt.c.
  >   o Only move BLOCK_BEG/BLOCK_END notes if optimize > 1 so as not
  >     to screw up debugging with -O1.
Sounds good.

  > May I check in the patch with those changes?
I think so.  Unless someone else immediately objects.

Jeff