Re: Your proposed change to loop.c

[Mark Mitchell <mark at codesourcery dot com>]

>>>>> "Joern" == Joern Rennecke <amylaar@cygnus.co.uk> writes:

    Joern> The loop that initialized stats[i].start_luid,
    Joern> stats[i].end_luid and increments end_needs_computing uses
    Joern> the numbering from before the first sort.  By having v->ix
    Joern> point back with the same numbering scheme, find_life_end
    Joern> could properly function, but the results were just garbled.
    Joern> I suppose you can fix that by setting i to v->ix instead of
    Joern> incrementing it.

Now we're getting somewhere.  This is the kind of information I'm
looking for.  Could you just go into a bit more detail?  I think you
forgot the part about me not being a loop.c master wizard. :-) Just
spell it out a bit more.

As I understand you:

  o Fisrt we compute v->ix:

    /* Initialize stats and set up the ix field for each giv in stats to name
       the corresponding index into stats.  */

  o Then we sort stats:

    qsort (stats, giv_count, sizeof(*stats), cmp_recombine_givs_stats);

  o Now, v->ix is wrong; we just changed all the indices around.  

I can see that this could cause general chaos.  Do I have it right so
far?  Is that the bug?

If so, isn't the fix just to iterate through assigning to v->ix after
the sort, instead of before?  Presumably, when we sort again a bit
later, we should do this again?  I think you're suggesting something
like this in your last line above, but I don't quite get what you
mean.  Go slow, work with me. :-)

That's not to say that your patch isn't good *for some other reason*.
Right now, I'm concentrating on getting the tree back in order; then
we can figure out what to do with your other improvements.

    >> In any case, wouldn't you agree that you have a responsibility
    >> to test each check-in?  In other words, before you check in a
    >> patch, apply just that patch to an otherwise virgin tree and
    >> test?  I can't tell from your comments whether you did this, or
    >> whether you tested a much larger patch sometime back, and just
    >> recently applied a little piece of it.

    Joern> I don't see much point in checking a toolchain with known
    Joern> bugs, when there are already fixes available.  You'll just
    Joern> end up finding the same bugs over and over again, and
    Joern> unable to do anything.

I can't disagree more vehemently.  The toolchain will always have
known bugs.  Our goal, however difficult to attain, is monotonic
progress: things should get better, and nothing should get worse.
That's why people are supposed to run bootstraps and regression tests
before checkins.  It's really not fair to everyone else working on
EGCS to randomly screw up the tree, without even testing.  The tree is
going to get screwed up every now and then: that happens.  But,
there's no excuse for not making a good faith effort to prevent that.

The point is that you checked in a patch that broke many, many
targets.  Unless I misunderstand you (and if I have, please consider
my objection withdrawn), you didn't test that patch *in isolation* on
*any* target.  As I understand it, this is a requirement for patch
submissions, and we have gone through this before.

    Joern> It looks for the actual time a giv is used, discounting any
    Joern> uses that will go away because of other givs that are to be
    Joern> reduced.  The code that loop churns out is geared to be
    Joern> worked on later by regmove.  When we have a series of
    Joern> derived givs with non-overlapping lifetimes in the same
    Joern> basic block (as in first to last use, the time they are set
    Joern> within the block is irrelevant), they will usually end up
    Joern> in a single register.

That sounds like a good improvement.  But, I'm trying to figure out
how to fix this bug, and I don't see why making regmove's life easier
is directly pertinent.  One thing at a time, please.

    Joern> Oh, I've worked on it for quite a while.  So are you
    Joern> proposing to put an alternate loop.c in the tree?  Or
    Joern> putting two sets of code in the same file?

There are various solutions, depending on how much the code has
changed.  Conditional compilation in the same file, run-time selection
in the same file, a separate file, a branch.

My point is that if you're going to redesign our loop-optimization
phase, there's not *necessarily* a reason we should look at every line
in your changes, or integrate each one piecemeal.  You could present
us with a finished product, complete with design information, let
everyone test it and comment on it, and then check it in wholesale,
just as you would probably do if you wrote an entirely new
optimization pass.  But, if you've only changed some parts here and
there, that's probably not the right thing to do, and the piecemeal
approach is probably best.

Note that I'm speaking a bit out of turn: I've only been deputized to
deal with this one issue.  What to do with all of your changes in the
long-term should be left to Jeff, or someone else with jurisdiction
over the whole tree.

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