PVS-Studio complaining about float comparison

Question

I scanned my code with PVS Studio analyzer and I am confused on why this error and how to fix this.

V550 An odd precise comparison: * dest == value. It's probably better to use a comparison with defined precision: fabs(A - B) < Epsilon.

bool PipelineCache::SetShadowRegister(float* dest, uint32_t register_name) {
    float value = register_file_->values[register_name].f32;
    if (*dest == value) {
        return false;
    }
    *dest = value;
    return true;
}

I am guessing to change code like this:

bool PipelineCache::SetShadowRegister(float* dest, float* epsilon uint32_t register_name) {
    float value = register_file_->values[register_name].f32;
    return fabs(dest - value) < epsilon;
}

Answer 1

Whoever's wondering, we're talking about this code.

I'll try to explain what PVS studio developers were trying to achieve with this message. Citing their reference about V550:

Consider this sample:

double a = 0.5;
if (a == 0.5) //OK
    x++;

double b = sin(M_PI / 6.0);
if (b == 0.5) //ERROR
    x++;

The first comparison 'a == 0.5' is true. The second comparison 'b == 0.5' may be both true and false. The result of the 'b == 0.5' expression depends upon the processor, compiler's version and settings being used. For instance, the 'b' variable's value was 0.49999999999999994 when we used the Visual C++ 2010 compiler.

What they are trying to say, is, comparing floating point numbers is tricky. If you just assign your floating point number, store it and move it around the memory to later compare with itself in this function - feel free to dismiss this error message.

If you are looking to perform some bit-representation check (which I honestly think you are doing), see below.

If you are performing some massive calculations on floating point numbers, and you are a game developer, calculating the coordinates of the enemy battlecruisers - this warning is one of your best friends.

---

Anyway, let's return to your case. As it usually happens with PVS-Studio, they did not see the exact error, but they pointed you to the right direction. You actually want to compare two float values, but you are doing it wrong. The thing is, if both float numbers you are comparing contain NaN (even in the same bit representation), you'll get *dest != value, and your code will not work in the way you want.

In this scenario, you better reinterpret the memory under float * as uint32_t (or whatever integer type has the same size as float on your target) and compare them instead.

For example, in your particular case, register_file_->values[register_name] is of type xe::gpu::RegisterFile::RegisterValue, which already supports uint32_t representation.

As a side effect, this will draw the warning away :)