Re: [patch] Fix some pessimizations due to fixes for PRs 27144 and 27639

[Roger Sayle <roger at eyesopen dot com>]

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
--