EPICS Math tests failing on Windows with MSVC

[Freddie Akeroyd]

Some epics base tests involving NaN and Inf have been failing for most windows MSVC builds, I have written a small test program https://github.com/FreddieAkeroyd/Testing/blob/master/src/mytests/inftest.cpp with output at https://ci.appveyor.com/project/FreddieAkeroyd/testing which illustrates the underlying cause.

The general summary is that:

VS2010, VS2017 and VS2019 are OK in all configurations
For other Visual studio versions:
    Debug builds are often OK
    All release builds fail.

The failure mode is as follows (Inf shown here but applies to NaN too):

Inf – Inf is evaluated correctly
Inf + -Inf is evaluated incorrectly
Inf + (-Inf) is evaluated correctly on VS 11.0 and 12.0, but not 14.0

The test code is using expressions like “a + -a” with a = Inf, if it instead tests “a + -b” with a = b = Inf then release mode works in all cases. So it looks like the repeated variable name is causing an undesired optimisation. I feel expressions of the form “a + -b” rather than “a + -a” are a better reflection of what is likely in practice, if that change is acceptable then I would be happy to submit a PR to use different variable names within binary operation tests

Regards,

Freddie

[mdavidsaver (mdavidsaver)]

Taking a step back. I ask myself, what is epicsMathTest actually testing? To me it's about runtime behavior. "Inf + - Inf" is nonsense which wouldn't be found in a real source file, so the fact that the MSVC optimizer gets it wrong is interesting, but ultimately a distraction.

While not the best solution. Absent fine grained control of the optimizer, maybe the path forward is to disable optimizations (only) in epicsMathTest.c and/or epicsCalcTest.c ? Or at least around the NaN and Inf tests?

> #if defined(_MSC_VER)
> # pragma optimize("g", off)
> #endif

[Freddie Akeroyd (freddie-akeroyd)]

Hi Michael,

I agree that testing "Inf + -Inf" directly is not useful, and might even suggest removing that test as written rather than adding a compiler pragma. If we only check this in debug builds, and users will be running release builds, then I feel some of the usefulness of the test is lost.

So thinking again about epicsMathTest as you say, what we probably care about is an expression like "a + -b" being evaluated in code/calc record where a or b (or both) might be "Inf". If we rewrite epicsMathTest to do these sorts of tests rather than a direct "Inf + -Inf" etc. then I feel it is really testing what we care about, and currently all MSVC compiler versions pass tests written in this style.

Regards,

Freddie

-----Original Message-----
From: <email address hidden> <email address hidden> On Behalf Of mdavidsaver
Sent: 29 September 2019 18:28
To: Akeroyd, Freddie (STFC,RAL,ISIS) <email address hidden>
Subject: [Bug 1845854] Re: EPICS Math tests failing on Windows with MSVC

Taking a step back. I ask myself, what is epicsMathTest actually testing? To me it's about runtime behavior. "Inf + - Inf" is nonsense which wouldn't be found in a real source file, so the fact that the MSVC optimizer gets it wrong is interesting, but ultimately a distraction.

While not the best solution. Absent fine grained control of the optimizer, maybe the path forward is to disable optimizations (only) in epicsMathTest.c and/or epicsCalcTest.c ? Or at least around the NaN and Inf tests?

> #if defined(_MSC_VER)
> # pragma optimize("g", off)
> #endif

--
You received this bug notification because you are subscribed to the bug report.
https://bugs.launchpad.net/bugs/1845854

Title:
  EPICS Math tests failing on Windows with MSVC

Status in EPICS Base:
  New

Bug description:
  Some epics base tests involving NaN and Inf have been failing for most
  windows MSVC builds, I have written a small test program
  https://github.com/FreddieAkeroyd/Testing/blob/master/src/mytests/inftest.cpp
  with output at https://ci.appveyor.com/project/FreddieAkeroyd/testing
  which illustrates the underlying cause.

  The general summary is that:

  VS2010, VS2017 and VS2019 are OK in all configurations
  For other Visual studio versions:
      Debug builds are often OK
      All release builds fail.

  The failure mode is as follows (Inf shown here but applies to NaN
  too):

  Inf – Inf is evaluated correctly
  Inf + -Inf is evaluated incorrectly
  Inf + (-Inf) is evaluated correctly on VS 11.0 and 12.0, but not 14.0

  The test code is using expressions like “a + -a” with a = Inf, if it
  instead tests “a + -b” with a = b = Inf then release mode works in all
  cases. So it looks like the repeated variable name is causing an
  undesired optimisation. I feel expressions of the form “a + -b” rather
  than “a + -a” are a better reflection of what is likely in practice,
  if that change is acceptable then I would be happy to submit a PR to
  use different variable names within binary operation tests

  Regards,

  Freddie

To manage notifications about this bug go to:
https://bugs.launchpad.net/epics-...

[Dirk Zimoch (dirk.zimoch)]

Maybe it is more interesting what a calc record is doing when asked to add infinities rather than to write constant expressions that the compiler may try to resolve at compile time.

[mdavidsaver (mdavidsaver)]

Most of the tests being performed in epicsCalcTest are cross-checks between the C compiler and the calc engine.

> /* Test an expression that is also valid C code */
> #define testExpr(expr) testCalc(#expr, expr);

[mdavidsaver (mdavidsaver)]

I'll try to summarize the outcome of the core meeting last week. For epicsCalcTest the decision was to remove the cross-check part of the test, and only check the the CALC express has the expected. For epicsMathTest, if was felt that trying to trick the MSVC optimizer just to make the tests pass wasn't worthwhile, so these are marked TODO for the compiler.

[Andrew Johnson (anj)]

Not sure what the state of this bug is, @mdavidsaver please change the Milestone to 7.0.3.2 (or remove it) if something still needs fixing, or set the Status to Fix Released if 7.0.3.1 includes all intended fixes.