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?
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
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
-- /bugs.launchpad .net/bugs/ 1845854
You received this bug notification because you are subscribed to the bug report.
https:/
Title:
EPICS Math tests failing on Windows with MSVC
Status in EPICS Base:
New
Bug description: /github. com/FreddieAker oyd/Testing/ blob/master/ src/mytests/ inftest. cpp /ci.appveyor. com/project/ FreddieAkeroyd/ testing
Some epics base tests involving NaN and Inf have been failing for most
windows MSVC builds, I have written a small test program
https:/
with output at https:/
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: /bugs.launchpad .net/epics- base/+bug/ 1845854/ +subscriptions
https:/