[patch] Fix some pessimizations due to fixes for PRs 27144 and 27639
Roger Sayle
roger@eyesopen.com
Sun Jun 18 02:42:00 GMT 2006
Hi Zdenek,
On Wed, 14 Jun 2006, Daniel Berlin wrote:
> Zdenek Dvorak wrote:
> > * tree-ssa-loop-niter.c (implies_nonnegative_p): New function.
> > (derive_constant_upper_bound): Derive more precise upper bound in
> > common cases. Return type changed to double_int.
> > (record_estimate): Reflect the changed return type of
> > derive_constant_upper_bound.
> > * double-int.c (double_int_zext, double_int_sext): Fix.
> >
> > * gcc.dg/tree-ssa/loop-18.c: New test.
Unfortunately, this patch causes a regression of builtins-14.c on (at
least) i686-pc-linux-gnu. The problem is that the compiler miscompiles
itself, incorrectly optimizing the loop in real.c:div_significands.
Because you claim to have bootstrapped and regression tested on
i686, I supect that it wasn't the bootstrapped compiler that you
used for regression testing [a stage1 compiler regtests fine].
A slightly more full explanation is that div_significands returned
inexact == 0 for 1.0/8.0 proir to your change, where as after
subversion revision 114674 (identified by hunting), we now return
inexact != 0. This is sufficient to cause the middle-end to think
that pow(2.0,-3.0) potentially requires FP rounding at run-time,
which causes the second of the two tests in builtins-14.c to fail
to be optimized. I suspect all compile-time floating point divisions
are currently being miscompiled, but builtins-14.c is one of the few
testcases checking this part of the compiler.
Looking at the problematic loop, it has all the "red flags" for
being mis-compiled by tree-ssa-loop-niter.c:
int i, bit = SIGNIFICAND_BITS - 1;
...
msb = 0;
goto start;
do
{
msb = u.sig[SIGSZ-1] & SIG_MSB;
lshift_significand_1 (&u, &u);
start:
if (msb || cmp_significands (&u, b) >= 0)
{
sub_significands (&u, &u, b, 0);
set_significand_bit (r, bit);
}
}
while (--bit >= 0);
...
So a jump into a loop, that can also be threaded, in a loop
with a predecrement in the loop condition, where the number
of iterations should be calculable at compile-time, and the
actual loop test being ">= 0", where your patch introduces a
new implies_nonnegative_p function. [People who work in
avionics wouldn't risk writing a loop like this! :>]
Whilst your patch may be innocently uncovering a latent bug
elsewhere in the compiler, there are so many corner-cases in
the problematic loop above, that I thought I'd let you investigate
from here.
Many thanks in advance.
p.s. the double-int bug fixes could have been committed independently
of the tree-ssa-loop-niter.c changes which would help rule out their
involvement, and/or simplify reverting your patch should that become
necessary.
Roger
--
More information about the Gcc-patches
mailing list