[Bug middle-end/82004] [8 Regression] SPEC CPU2017 628.pop2_s miscompare

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82004

--- Comment #9 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 5 Sep 2017, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82004
> 
> Jakub Jelinek <jakub at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |jakub at gcc dot gnu.org
> 
> --- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> (In reply to Thomas Koenig from comment #7)
> > (In reply to Richard Biener from comment #6)
> > 
> > > I suppose reporting this to SPEC folks and requesting a chlcnc table
> > > change to .009999_r8 isn't going to fly ;)
> > 
> > I think reporting this to be a good idea.
> > 
> > All the SPEC people should have read "What Every Computer Scientist
> > Should Know About Floating-Point Arithmetic", right? :-)
> 
> They often don't care though, something that happened e.g. with out of bound
> array accesses in SPEC2k6.  But maybe they will when SPEC2k17 is new.

Note in this case we have some really weird code anyway:

   integer (int_kind), parameter :: &
      nsub = 400,  &! total number chlorophyll concentrations in look up 
table
      ksol = 2*km, &! (0:ksol) transmission levels (layer interface + 
mid-point)
      nchl = 31     ! number of chlorophyll absorption coefficients

...
   real (r8), parameter, dimension(nchl) :: &!chl for table look-up
     chlcnc = (/                            &
     .001_r8, .005_r8, .01_r8,  .02_r8,     &
     .03_r8,  .05_r8,  .10_r8,  .15_r8,     &
     .20_r8,  .25_r8,  .30_r8,  .35_r8,     &
     .40_r8,  .45_r8,  .50_r8,  .60_r8,     &
     .70_r8,  .80_r8,  .90_r8, 1.00_r8,     &
     1.50_r8, 2.00_r8, 2.50_r8, 3.00_r8,    &
     4.00_r8, 5.00_r8, 6.00_r8, 7.00_r8,    &
     8.00_r8, 9.00_r8,10.00_r8  /)

...
   chlmin  = chlcnc(1)
   chlmax  = chlcnc(nchl)
   dlogchl = (log10(chlmax)-log10(chlmin))/real(nsub)
...
   logchl = log10(chlmin) - dlogchl
   do n=0,nsub
     logchl = logchl + dlogchl
     chlamnt = 10**(logchl)
     do m=1,nchl-1
       if (chlcnc(m) .le. chlamnt .and. &
           chlamnt .le. chlcnc(m+1) ) then
      mc = m

so we start with 10**-3.  We'll eventually even see that in PRE
but likely exactly evaluate the starting FP constant.

What is fishy here is that the code compares

chlcnc(m) <= chlamnt && chlamnt <= chlcnc(m+1)

that is, both <=.  This is likely _just_ to make the first entry
match.  And the code relies on log10 and 10** exactly
cancelling for the .001_r8 entry.

So yes, I think a fix is warranted.

As a last resort we can always choose to not touch 10**x
but only transform it to exp10 when available (similar for
2**x and e**x -- I doubt log(e) computes 1 "exactly enough"
though I hope math.h has M_E and M_El that when fed to
mpfr log will compute 1.0).