Add VS2019 to CI

[bernhardmgruber]

This PR adds Visual Studio 2019 to the CI.

[bernhardmgruber]

Hi @amyspark! Since you seem to be pretty knowledgeable on SIMD programming and the MSVC toolchain, you may have an idea what breaks the Visual Studio 2019 builds here. There are only two failures:

• one is in Vc::exp having a larger error now. You had a look at Vc::exp recently as well, so I figured you might also know what is going on here. It might also just be different floating point optimizations done by MSVC. The new results are a few ULPs off.
• the other is a utility function. And it just fails for SSE.

If you have any clue on what is happening here, I would be super happy! Thank you very much!

[amyspark]

@bernhardmgruber Vc::exp's float implementation is a direct port of the double-precision one, which needs (to begin with) an adjustment to its boundary values. (This is in #295.) The rest, I can have a look at them later!

[amyspark]

@bernhardmgruber, re the Vc::exp failures, it seems there's an issue with either the implementation on common/exponential.h, Vc::ldexp, or both. On my end, I've needed to enable /fp:strict and replace the ldexp call with the following, to make those tests pass:

        for (size_t i{0}; i != x.size(); i++) {
            x[i] = z[i] * std::powf(2, n[i]);
        }
        // x = ldexp(z, n); // == z * 2ⁿ

I'll continue later with the utils error.

[amyspark]

@bernhardmgruber codegen bug, please insert AddCompilerFlag("/d2SSAOptimizer-") in the test suite flags for MSVC. Reported here:

• AVX codegen: https://developercommunity.visualstudio.com/t/AVX-codegen-bug-on-Vc-with-MSVC-2019/1470844
• SSE codegen: https://developercommunity.visualstudio.com/t/SSE-codegen-bug-on-Vc-with-MSVC-2019/1499725

[amyspark]

@bernhardmgruber, can you trigger a CI check again? I've been able to confirm Microsoft's assertion that the SSE bug is fixed in 16.10.4.

[amyspark]

I see we now have a clean fail on MSVC, I'll check on those during the week.

[amyspark]

AVX failures are confirmed and already marked as "under investigation" by MS.

With the same compiler version, I cannot reproduce the SSE failures here. I can only think it may be down to the runner's CPU vendor -- @bernhardmgruber, do you have an Intel box to test with, since mine is a Ryzen 1st gen?

[amyspark]

Note that the AppVeyor runs do complete successfully, although with a slightly older MSVC; it's another inconsistency to tally...

[bernhardmgruber]

Note that the AppVeyor runs do complete successfully, although with a slightly older MSVC; it's another inconsistency to tally...

AppVeyor completes successfully, because it builds with VS2017. Btw.: I removed AppVeyor (and travis) from the CI in #293.

[bernhardmgruber]

I am starting to wonder if the failing unit tests can ever be correct. There is no requirement on the precision of std::exp. I just dumped the reference results of the failing test math_avx.testExp, which calls std::exp on random numbers, into text files with VS2019 and GCC11. They are different. So if the implementation of std::exp is different in libstdc++ than in MSVC, how can a single Vc::exp compare equal to the reference results with two different standard libraries?

Maybe we should just declare that Vc tries to reproduce the results of libstdc++ and set much higher epsilons in the unit tests when a different standard library is used.

[amyspark]

I am starting to wonder if the failing unit tests can ever be correct. There is no requirement on the precision of std::exp. I just dumped the reference results of the failing test math_avx.testExp, which calls std::exp on random numbers, into text files with VS2019 and GCC11. They are different. So if the implementation of std::exp is different in libstdc++ than in MSVC, how can a single Vc::exp compare equal to the reference results with two different standard libraries?

Maybe we should just declare that Vc tries to reproduce the results of libstdc++ and set much higher epsilons in the unit tests when a different standard library is used.

If any precision issues arise with MSVC (especially in the presence of /fp:precise and lower), those should be reported ASAP. I know they've been running into many bugs with their SSA optimizer in 2019.

[bernhardmgruber]

If any precision issues arise with MSVC (especially in the presence of /fp:precise and lower), those should be reported ASAP. I know they've been running into many bugs with their SSA optimizer in 2019.

As I said, std::exp does not have a guaranteed precision, so if MSVC changes what they return here, it is technically not a bug from the viewpoint of the standard :) There might still be a bug in their implementation of std::exp though, but how can I judge? They might just as well have improved the implementation of std::exp from VS2017 to VS2019. We could consider reporting this. I tried to build a case on godbolt.org, but unfortunately I cannot run binaries from MSVC there :/

[amyspark]

If you can share the Godbolt, I can download and run it here. I've got 2017, 2019, the 2022 preview in my box, and also various MinGW flavors for further comparison.

[bernhardmgruber]

If any precision issues arise with MSVC (especially in the presence of /fp:precise and lower), those should be reported ASAP. I know they've been running into many bugs with their SSA optimizer in 2019.

I reached the point where I really start to believe this can be a compiler bug now. I tried printing the constants where the tests would fail and ended up here:

int main() {
    const auto v = 0x1.26f8f4p+3f; // 9.2178897857666015625f
    const auto r = std::exp(v);
    printf("%f %a", r, r);
    // gcc               10075.780273
    // VS2019            10075.780273

    const auto vv = Vc::Vector<float, Vc::VectorAbi::Avx>(v);
    const auto rr = Vc::exp(vv);
    printf("%f %a", static_cast<float>(rr[0]), static_cast<float>(rr[0]));
    // gcc               10075.780273
    // VS2019            10075.780273
}

That is all correct. But if the float value 0x1.26f8f4p+3f occurs in x/_x in the unit test math_avx/testExp<simd< float, AVX>>:

    for (size_t i = 0; i < 100000 / V::Size; ++i) {
        V x = (V::Random() - T(0.5)) * T(20);
        V reference = x.apply([&b](T _x) { return std::exp(_x); });
        FUZZY_COMPARE(Vc::exp(x), reference) << ", x = " << x << ", i = " << i;
    }
    // VS2019 unit tests Vc::exp = 10075.77539, reference = 10075.78027

the result is wrong. While playing around, I introduced the following side effect:

    for (size_t i = 0; i < 100000 / V::Size; ++i) {
        V x = (V::Random() - T(0.5)) * T(20);
        V reference = x.apply([](T _x) {
            if (std::abs(_x - 9.21789) < std::numeric_limits<T>::epsilon() * 10)
                printf("");
            return std::exp(_x);
        });
        FUZZY_COMPARE(Vc::exp(x), reference) << ", x = " << x << ", i = " << i;
    }
    // VS2019 test passes

This should not change the result, but it somehow makes the test pass. So I guess the call to printf somehow puts a barrier into the instruction flow for the optimizer and the bug is not triggered.

I don't know how to continue from here. @amyspark if you want you can try to reproduce this even better and file a bug report to MS. Otherwise, I am also fine to just live with the issue and increase the error margins for MSVC in the unit tests. We can revisit this in the future and see if a future version of VS fixes this.

[amyspark]

@bernhardmgruber I'll certainly work on a test case for MS. Did you check with the /d2SSAOptimizer- flag? You could add it to the tests if(MSVC AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 19.20).

[bernhardmgruber]

Did you check with the /d2SSAOptimizer- flag?

I just tried that and indeed, the tests pass successfully then.

[amyspark]

I just updated the bug report at Microsoft's with the new test case.

[amyspark]

LGTM 🎉

[amyspark]

@bernhardmgruber, Karen Huang from Microsoft needs further information as to the processor's specs you are using.
As I cannot reproduce this anymore on my Ryzen box, perhaps you can comment in the bug report?

[bernhardmgruber]

@amyspark I commented on the bug report. Thx raising my awareness!