Re: Withdrawn patches

[Richard Sandiford <rsandifo at redhat dot com>]

Roger Sayle <roger@eyesopen.com> writes:
> > Err... the first one did.  Search for gcc.c-torture/execute/20030916-1.c
> > in the message linked above.  And the problem that the testcase exposes
> > is described in the covering message.
> 
> My apologies.  I completely missed that in the middle of the diff.  It
> wasn't mentioned in either ChangeLog and didn't appear at the end of the
> posting.

Well, in the original posting, there were several attachments.
The testcase was at the end of the first attachment, in the normal place.
But I realise that this isn't obvious from the archives.

> > The second one didn't because I didn't know about the bug at the
> > time either. ;)  It was originally submitted an optimisation,
> > not a bug fix.  But I think the message explained what the patch
> > was trying to achieve.
> 
> I certainly appreciate how difficult it is to write "portable" test
> cases for GCC's middle-end and RTL optimizers.  However, I'm just
> a bit cautious during stage3, and if it isn't possible to check that
> a patch is doing the correct thing, and that there isn't a bad latent
> interaction with some other pass or on another platform, then my
> personal opinion is that it can wait until 3.5 rather than risk
> destablizing the tree.

And if someone had said that, I'd have been happy to accept their
judgement.  But it seemed to me that the patch was just being ignored.

The same goes for much of the rest of your post.  I realise you only
became a middle-end maintainer recently and didn't have chance to
approve it in stage 2.  But if you have good reasons for punting on a
patch, please say so!  It's a lot nicer to know that your patch is being
left alone for good reasons rather than just being dropped on the floor.

> For example, only this morning you decided for similar reasons to
> revert your direct_pool_load_p patch.
> http://gcc.gnu.org/ml/gcc-patches/2003-12/msg00446.html

Yeah, I have a terrible track-record, especially recently, so perhaps I
shouldn't be surprised...

> Again neither this patch, nor its reversion included testcases,
> leaving mainline in the state that it won't obviously be clear
> that this problem was ever resolved and whether it still exists.

But here again, I did post example code.  And I posted the differences
in c-torture output for one configuration from each backend to show
why I thought the patch was an improvement in code quality.

OK, so I should have provided a dejagnu test case as well.  But I
dispute that such a testcase, with appropriate scan-assembler gubbins,
would have made the patch easier for a reviewer to evaluate, if that's
the point you're making.  The patch wasn't supposed to improve a
particular test case or fix a particular bug.  It was supposed to
lead to a general improvement in code quality.

> For me personally, a testcase that comes with a patch counts for a huge
> amount.  A new deja-gnu test, even if only on a single platform, clearly
> demonstrates that a bug has been fixed and that the patch is "obviously"
> suitable even during stage3 or on a release branch.

True, but I would never have posted these patches during stage 3 anyway ;)
I was originally hoping to have them accepted on the basis that they
were posted well before the start of stage 3.  But things dragged on,
and it became clear it was too late for that.

Richard