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

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

[mailboxnotfound at yahoo dot com]

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

john henning <mailboxnotfound at yahoo dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mailboxnotfound at yahoo dot com

--- Comment #42 from john henning <mailboxnotfound at yahoo dot com> ---
A comment, an invitation, and some questions from one of those "SPEC people".

The comment: 

SPEC CPU starts from real applications (rather than artificial kernels). 
Therefore, yes, it begins with code that includes practices that are less than
perfect - including standards violations and mistakes in numerical methods.  

SPEC then weeds out benchmark candidates when they are discovered to be
inadequate.  Many have been weeded out due to numerical malpractice, when they
cannot be relied upon to produce correct answers within some predefined
tolerance.  

The remaining candidates get pushed toward the standard, get pushed toward
better numerical practices.

But all this is driven by testing.   If there's nobody pushing hard on compiler
X advanced optimization feature Y, and if Y quietly relies on source code to be
virtuous as regards Z, it is entirely possible that sins against virtue Z will
escape notice until after release of a SPEC benchmark suite.

Pre-release, fixes are easy to sponsor and apply.   Post-release, it is much
harder, because of SPEC's preference for "no moving targets". 

The invitation:

Thus I would invite any of the individuals who commented about "those SPEC
people" to consider becoming one, and helping with the testing of benchmark
candidates for the next benchmark suite.  SPEC has a mechanism to accept free
volunteer labor :-) and if you would like to volunteer, please write to info at
SPEC dot org.  If you like, you could mention my name and this bug when you do.

The questions:

(q1) Do I correctly gather that the fix described here would have gone into GCC
8?

(q2) On aarch64 with GCC 8.2, GCC 9.3, and GCC 10.1 (Red Hat 7.8, glibc
2.17-307.0.2.el7.1) I am seeing the following 628.pop2_s miscompare and abort. 
Is this the same problem or a different problem as the one described in this
bug?

Miscompare:
   1513:    Chlorophyll transmission table computed
           Could not find range for chlamnt =  1.0000E-03

Abort: 

   Contents of pop2_s.err
   ****************************************
   Note: The following floating-point exceptions are signalling:
IEEE_UNDERFLOW_FLAG
   ------------------------------------------------------------------------
   POP aborting...
     set_chl_trans range error for chlamnt

(q3) The text mentions that -funsafe-math-optimizations as a workaround.  Was
this tested?  Was it narrowed to any of the components of
-funsafe-math-optimizations which apparently are -fno-signed-zeros,
-fno-trapping-math, -fassociative-math and -freciprocal-math?  

I am just about to start such testing on my aarch64 system, but if someone has
already done such, it would be useful to know.

(q4) *Is* there a proposed patch for 628.pop2_s?   Although such may be
unlikely to be applied to CPU 2017, it is conceivable that there might be an
updated version of 628.pop2_s in a future suite.  So, it would be good to have
the proposed patch on record.

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

[rguenth at gcc dot gnu.org]

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

--- Comment #43 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to john henning from comment #42)
> A comment, an invitation, and some questions from one of those "SPEC people".
> 
> The comment: 
> 
> SPEC CPU starts from real applications (rather than artificial kernels). 
> Therefore, yes, it begins with code that includes practices that are less
> than perfect - including standards violations and mistakes in numerical
> methods.  
> 
> SPEC then weeds out benchmark candidates when they are discovered to be
> inadequate.  Many have been weeded out due to numerical malpractice, when
> they cannot be relied upon to produce correct answers within some predefined
> tolerance.  
> 
> The remaining candidates get pushed toward the standard, get pushed toward
> better numerical practices.
> 
> But all this is driven by testing.   If there's nobody pushing hard on
> compiler X advanced optimization feature Y, and if Y quietly relies on
> source code to be virtuous as regards Z, it is entirely possible that sins
> against virtue Z will escape notice until after release of a SPEC benchmark
> suite.
> 
> Pre-release, fixes are easy to sponsor and apply.   Post-release, it is much
> harder, because of SPEC's preference for "no moving targets". 
> 
> The invitation:
> 
> Thus I would invite any of the individuals who commented about "those SPEC
> people" to consider becoming one, and helping with the testing of benchmark
> candidates for the next benchmark suite.  SPEC has a mechanism to accept
> free volunteer labor :-) and if you would like to volunteer, please write to
> info at SPEC dot org.  If you like, you could mention my name and this bug
> when you do.

Thanks.  But all testing effort is limited.  We do test SPEC before the
release (but I am not aware that sources are available freely before
release - I mean, including the data and scripts).  But the issue required
flags to be reproduced that we didn't test in time.  A full review of
the source packages is of course not possible.

> The questions:
> 
> (q1) Do I correctly gather that the fix described here would have gone into
> GCC 8?

Yes.

> (q2) On aarch64 with GCC 8.2, GCC 9.3, and GCC 10.1 (Red Hat 7.8, glibc
> 2.17-307.0.2.el7.1) I am seeing the following 628.pop2_s miscompare and
> abort.  Is this the same problem or a different problem as the one described
> in this bug?
> 
> Miscompare:
>    1513:    Chlorophyll transmission table computed
>            Could not find range for chlamnt =  1.0000E-03
> 
> Abort: 
> 
>    Contents of pop2_s.err
>    ****************************************
>    Note: The following floating-point exceptions are signalling:
> IEEE_UNDERFLOW_FLAG
>    ------------------------------------------------------------------------
>    POP aborting...
>      set_chl_trans range error for chlamnt

It looks like the same issue, yes.

> (q3) The text mentions that -funsafe-math-optimizations as a workaround. 
> Was this tested?  Was it narrowed to any of the components of
> -funsafe-math-optimizations which apparently are -fno-signed-zeros,
> -fno-trapping-math, -fassociative-math and -freciprocal-math?  
> 
> I am just about to start such testing on my aarch64 system, but if someone
> has already done such, it would be useful to know.

I'm not aware that anyone tried to narrow down a workaround.

> (q4) *Is* there a proposed patch for 628.pop2_s?   Although such may be
> unlikely to be applied to CPU 2017, it is conceivable that there might be an
> updated version of 628.pop2_s in a future suite.  So, it would be good to
> have the proposed patch on record.

>From what I understand the proposed workaround is something like the
following if not rewriting the problematic loop in another way.

diff --git a/benchspec/CPU/628.pop2_s/src/sw_absorption.F90
b/benchspec/CPU/628.pop2_s/src/sw_absorption.F90
index 1aa25904..489e88f2 100644
--- a/benchspec/CPU/628.pop2_s/src/sw_absorption.F90
+++ b/benchspec/CPU/628.pop2_s/src/sw_absorption.F90
@@ -134,7 +134,7 @@

    real (r8), parameter, dimension(nchl) :: &!chl for table look-up
      chlcnc = (/                            &
-     .001_r8, .005_r8, .01_r8,  .02_r8,     &
+     .00099999999_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,     &

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

[jakub at gcc dot gnu.org]

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

--- Comment #44 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Sorry if my comment sounded harsh.
Anyway, the older issue I was referring to was
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53073 with SPEC2k6 464.h264ref.

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

[mailboxnotfound at yahoo dot com]

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

--- Comment #45 from john henning <mailboxnotfound at yahoo dot com> ---
I had promised to do some more testing.  

There were miscompares during the training run (thus causing the build to fail)
using

     -Ofast -flto -fprofile-generate/-fprofile-use 

on aarch64 with GCC 8.2, 9.3, and 10.1 and
on x86_64  with GCC 8.3, 9.3, and 10.1.

The above succeeded with GCC 7.3 on both aarch64 and x86_64.

The attachment 628.pop2.moretests.7aug2020.txt digs into 10.1 a bit deeper
(posted as an attachment to reduce the chances of unwanted line wraps or white
space mangling for the tables)

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

[mailboxnotfound at yahoo dot com]

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

--- Comment #46 from john henning <mailboxnotfound at yahoo dot com> ---
Created attachment 49027
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49027&action=edit
More testing 7-aug-2020

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

[mailboxnotfound at yahoo dot com]

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

--- Comment #47 from john henning <mailboxnotfound at yahoo dot com> ---
SPEC next step: Because the performance differences were small (in my limited
testing) no matter which workaround I picked (-O3, or remove Feedback Directed
Optimization, or add -fno-unsafe-math-optimizations), I will probably update
the CPU 2017 Docs to recommend simply that GCC users set "basepeak" for
628.pop2. This turns off FDO, and is easy for them to do. 
(https://www.spec.org/cpu2017/Docs/config.html#basepeak)

A loose end: is it worth filing a documentation bug for the fact that using the
alleged components of -fno-unsafe-math-optimizations does not have the same
effect as -fno-unsafe-math-optimizations itself?  (See the attachment from
comment 46)