Your proposed change to loop.c
Wed Jun 23 13:02:00 GMT 1999
With Jeff's permission, I've been trying to review
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,
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
- ??? 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.
Mark Mitchell email@example.com
CodeSourcery, LLC http://www.codesourcery.com
More information about the Gcc-patches