Re: Withdrawn patches

[Richard Sandiford <rsandifo at redhat dot com>]

Roger Sayle <roger@eyesopen.com> writes:
> > > Unsafe givs and check_ext_dependent_givs:
> > > http://gcc.gnu.org/ml/gcc-patches/2003-09/msg01268.html
> >
> > Now filed as #13299: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13299
> >
> > > Promoted bivs:
> > > http://gcc.gnu.org/ml/gcc-patches/2003-09/msg01293.html
> >
> > Now filed as #13300: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13300
> 
> Well that's unfortunate!  I'd read through both of these patches but
> it wasn't clear if they were only addressing hypothetical latent bugs.
> Neither included a testcase for GCC's regression suite nor mentioned a
> bugzilla PR.

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.

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.

Then geoffk commented on the patch and pointed out that the
current SIGN_EXTEND assumptions weren't in fact correct.

> This of course, makes it much harder for a reviewer to convince
> himself that a patch does what it claims to, and/or fixes an
> observable defficiency in the compiler.

Sure.  But it's difficult to provide dejagnu tests for things like
strength reduction.  It's not like builtins, fold() or gcse, where
we can use tricks like link_error().

So instead I tried to provide an example of the sort of loop that the
optimisation would affect, and show the before and after code on my
favourite target.  That's fixing an observable deficiency, just like
dejagnu tests do.

> Once again, its a shame that you had to withdraw these patches before
> providing either PR numbers or a testcases.  They'd have been approved
> almost immediately if you'd included this information in your original
> postings.  C'est la vie.

Not sure I understand what you mean here.  It sounds like, if I'd
filed a bugzilla report saying "loop optimisers don't recognise
promoted bivs", and cut&paste the covering message for my second
patch into it, the patch would somehow have been easier to review
than it was by including the description with the patch itself...

Richard