Your proposed change to loop.c

[Mark Mitchell]

Joern --

  With Jeff's permission, I've been trying to review
http://egcs.cygnus.com/ml/egcs-patches/1999-06/msg00484.htm .

I've got several questions.  First, the easy ones:

+   unsigned leading_combined : 1;/* In recombine_givs, set if this giv has been
+                                  combined with one or more other givs that
+                                  precede the giv insn of this giv.
+                                  Giv derivation then requires to move the
+                                  giv insn before the first use.  */

There's a typo in that comment: "requires to move".  Perhaps "requires
us to move"?

*************** record_giv (v, insn, src_reg, dest_reg, 
*** 5422,5427 ****
--- 5428,5434 ----
    v->auto_inc_opt = 0;
    v->unrolled = 0;
    v->shared = 0;
+   v->leading_combined = 0;
    v->derived_from = 0;
    v->last_use = 0;

Why isn't live_after_loop initialized here?  Even if it is actually
always initialized somewhere else, I would prefer it initialized here,
for clarity.

Now, the hard questions:

  o I don't understand what the bug is in find_life_end that is 
    causing the current endless loop.  Please explain it to me,
    bearing in mind that I am not an long-time expert loop.c hacker
    like yourself!  (Of course, I do know about compilers in general, 
    and GCC in particular, and understand loop optimizations.)  I
    don't find the loop.c comments terribly helpful (not the ones
    in your patch, the ones already in the code.)

    I'd also like to know on which system your original change was
    bootstrapped, and why the bug didn't appear there.  That would
    probably help me understand what's going on a little better.

  o If the above has not yet made it obvious, please explain why
    there is not a simpler fix.  Is there a bug in find_life_end?
    If not, is the code that uses it not treating its return value
    correctly?  It looks to me like the point of live_after_loop
    is to enable moving instructions around.  You removed this code:

-             /*  Check that the giv insn we're about to use for deriving
-                 precedes all uses of that giv.  Note that initializing the
-                 derived giv would defeat the purpose of reducing register
-                 pressure.
-                 ??? We could arrange to move the insn.  */
-             && ((unsigned) stats[i].end_luid - INSN_LUID (loop_start)
-                   > (unsigned) stats[i].start_luid - INSN_LUID (loop_start)

    That suggests to me that perhaps you're doing some additional 
    optimization that we were not doing before?  Can we not do this
    extra optimization, but still fix the bug?

  o If there's no simpler fix, why does this fix the bug?  What's the
    top-level summary of your approach?  Did you bootstrap/test this
    change on one of the systems affected by the current problem?

I think I'll back the recent loop.c patches out, back to 1.162, except
for Jeff's change in 1.164 tonight (PDT) if we can't get this sorted
out.  I bet, however, that your current patch is fine, and that we
can put it in.  I just need your help to understand it more clearly.

Thanks,

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com