VC++ using fp:fast causes wrong (not just inaccurate) results - is this a compiler bug?

user_192359 0
Viewed 1056

I've installed the latest VS2017 update (15.4.4), and when compiling our projects the unit tests started failing. It seems that the problem happens in some scenarios when using optimizations (/O2) and fast model for floating point (/fp:fast). This problem did not happen with a previous compiler (VS2017 update 15.2).

Here's a sample program: 

#include <iostream>

const float FACTOR = 0.01745329251994329576923690768489f;

unsigned long long hoursToMicrosecs(int hours)
{
    return hours * 3600 * 1000000LL;
}

float degToRad(float deg)
{
    return deg * FACTOR;
}

float f(int u1, int u2)
{
    float percent = ((u1 - u2) / 1000.0f) / (hoursToMicrosecs(24) / 1000000.0f);
    return degToRad(percent * 360);
}

int main()
{   
    auto result = f(3600000, 35063681);
    std::cout << result << '\n';
    return (result > -3.0f) ? 0 : -1;
}

result should be -2.2881, but the output is actually -394.868, which is not just inaccurate.

It works if I do any of the following:

  • Remove optimizations
  • Change to fp:precise
  • Return to the previous compiler (15.2)

Looking at the disassembly shows us that the compiler tried doing something nice for us - it just calculated the whole thing at compile time and replaced with a single number.

The optimized code is just a one-liner:

011F1000  vmovss      xmm0,dword ptr [__real@c3c56f11 (011F2118h)]

My question is: Is this a compiler bug (which I should report) or is it wrong usage of fp:fast?

compiler-bug visual-studio-2017 floating-point visual-c++ c++
1 Answers

I'm seeing the same thing here. I think I found the issue: hoursToMicroseconds(24) is overflowing. It shouldn't, as the expression should be converted to unsigned long long before the multiplication with 1000000LL. But if you cast that result back to an unsigned int, you'll get 500654080 rather than 86400000000. So the whole calculation eventually boils down to -394.868.

I would definitely say this is a compiler bug. It seems you can circumvent it by casting the unsigned long long result to double first.

EDIT

You know what else is funny? If you make all your functions constexpr, and also declare result as constexpr in main(), it will produce the right result. So there are apparently two separate code paths in the compiler that can compute the value at compile time, and one of them is broken.

user_89514730
Related