This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PING^(n+1) fwprop merge (ping 6 or 7 by now)


On 5/7/06, Roger Sayle <roger@eyesopen.com> wrote:

Hi Steven (and Paolo),


On Sun, 7 May 2006, Steven Bosscher wrote:
> >> fwprop merge, stage 2 project:
> >> http://gcc.gnu.org/ml/gcc-patches/2006-02/msg01420.html
> >> http://gcc.gnu.org/ml/gcc-patches/2006-02/msg01421.html
> Ping ping ping ping ping ping!!!!

Constructively, it would help significantly if someone updated these
patches to mainline, rebootstrapped and regression tested them and
resubmitted them.

I'm doing that as we speak. I have bootstrapped (multilib) the patches on x86_64 for c,c++,objc,fortran,ada,java and I'm waiting for the regression tests to complete. Then I'll post the updated patch with the test results (which should IMVHO be a requirement FWIW).


 There have been a significant number of changes
in the last few months, and it seems only reasonable to ensure that
significant changes in stage3 apply cleanly and don't cause regressions.

It would have been reasonable to don't let a patch go without any reply whatsoever for four months, too.

The updates are a mere 3 changes that affect these two patches:
- CLEANUP_PRE_LOOP is gone
- execute in the tree_pass structure now is unsigned int
- validate_change_maybe_volatile is gone so that one hunk doesn't apply


I'm not sure why Paolo isn't pinging these patches himself,

Paolo _has_ pinged his patches. Many times. And besides, I'm also an author of this new pass, so they are my patches too.

This is one reason why PINGs
by interested third parties (other than the original contributor)
carry slightly less weight.

RTFfwprop.c file and see my name on it so I don't consider myself a third party, thank you very much.


These submissions claim to be mostly performance neutral on SPEC, with a
slight hit for PPC,

No slight hit on PPC. I appreciate you reply to my ping, but it would have been helpful if you've read Paolo's complete story. The PPC regression was fixed already even before these fwprop patches were posted.

Personally, I'd love to see the 3% compile-time improvements from
disabling path-following in CSE provided that the patch is safe and
the performance impact on most platforms is negligible.  However,
there are already complaints about the reasoning used to accept/reject
significant changes in stage3, and whether they are truly ready for
GCC just becuase they are pinged continually.

First of all, this work was finished in september last year.


Second, it has been on a branch (the dataflow branch) for months now,
which has been tested on a large number of targets.

Third, this is not a stage3 contribution, and I can't help that the
reviewers don't do their job.

If I post a patch in stage2, and early stage2 too if I may add, then
you should not slap me with "significant changes in stage3", but
instead ask yourself "why didn't I take the responsibility that I have
as a reviewer to comment on the patch?". IMHO you've forfeited  your
right to complain about timing if a patch has been pending as long as
these two.

The postings for these patches also reported that these changes had
only been bootstrapped and regression tested on i686-pc-linux-gnu.
If someone was serious about their patches, they'd perhaps do slightly
more than the required minimum testing.

But they had previously been bootstrapped and tested by me on {i686,x86_64,ppc,ppc64,ia64}-suse-linux-gnu. Plus SPEC on the two x86* targets. Plus nullstone on x86*. Plus PPC testing by David Edelsohn. Plus on a number of targets on the dataflow-branch. That _is_ slightly more than the required minimum testing.

I realize you're not a mind reader, but when this patch was posted,
just testing on one target was enough for a patch contribution.  Now
you are raising the bar because, essentially, you and your fellow
reviewers have failed. That is the way I see it at least.

There's also the issue of breakage and fallout.  Paolo's most recent
patch is still causing mainline regressions on x86_64, see the post
http://gcc.gnu.org/ml/gcc-patches/2006-04/msg00390.html and notice
the "PING^n fwprop" in the title.

If you had read beyond the subject, you'd have seen that the regressions mentioned concern a reload change, which also had to be pinged because nobody is reviewing patches. That message you point to had nothing to do with fwprop other than the subject line.

Oh btw, I seem to recall that all your patches in the tree overflow
mess also have caused numerous regressions.  The difference between
yours and fwprops (if any, and I doubt it) is that you can approve
your own patches so that they are commited when they are ready, and
other people like Paolo and me have to wait until some reviewer is so
kind to take a little time to look at some patch. Which obviously
doesn't happen enough as proven by the huge number of pending patches
in e.g. the patch tracker. So you have time to fix whatever you've
broken, and we don't.

In fact, I'm fed up with your I-know-better attitude here.  Many of
the points you bring up have little or no relation to fwprop as far as
I can see, and none of them would have been particularly relevant if
these patches had been reviewed in time.

Your reply is the kind of message that makes me wonder why I try to
contibute to gcc at all.

Gr.
Steven


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]