[libcxx/test] Floating point comparisons

Clang Frontend
#1

Hi,

I am experimenting with libcxx on FreeBSD and got a lot of asserts on floating point comparisons, most notably in 'libcxx/test/containers/unord', e.g.:

Assertion failed: (c.load_factor() == (float)c.size()/c.bucket_count()), function main, file assign_move.pass.cpp, line 71.

Switching on optimization makes them go away. I'm guessing one of the values gets promoted from a float to a double somewhere, which makes the comparison fail.

The question is: do the lvalue and rvalue need to be exactly the same? I think it would be better to use something like:

assert(fabs(c.load_factor() - (float)c.size()/c.bucket_count()) < FLT_EPSILON);

to sidestep the issue. It also has the benefit of being more implementation independent?

Regards, Ed.

#2

Before changing this I'd like to understand the issue, just to make sure we wouldn't be hiding a bug.

load_factor() is computed (in <__hash_table>) as:

    _LIBCPP_INLINE_VISIBILITY float load_factor() const _NOEXCEPT
    {
        size_type __bc = bucket_count();
        return __bc != 0 ? (float)size() / __bc : 0.f;
    }

which is precisely how the test is computed too. So I currently do not understand the cause of the assert.

And just to head off the discussion that one should never compare floating point with equality: I'm aware of how floating point works. But I also know that it is deterministic, down to the very last bit.

Is there a legal compiler transformation that can make this test fail?

Howard

#3

Before changing this I'd like to understand the issue, just to make sure we wouldn't be hiding a bug.

load_factor() is computed (in <__hash_table>) as:

   _LIBCPP_INLINE_VISIBILITY float load_factor() const _NOEXCEPT
   {
       size_type __bc = bucket_count();
       return __bc != 0 ? (float)size() / __bc : 0.f;
   }

which is precisely how the test is computed too. So I currently do not understand the cause of the assert.

And just to head off the discussion that one should never compare floating point with equality: I'm aware of how floating point works. But I also know that it is deterministic, down to the very last bit.

Is there a legal compiler transformation that can make this test fail?

Howard

#4

Not so fast. See C++98[expr]p10 / C++11[expr]p11, "The values of the
&#64258;oating operands and the results of &#64258;oating expressions may be
represented in greater
precision and range than that required by the type". There's no requirement
that the compiler do this in a consistent way, or even do it the same way
every time the same expression is evaluated.

(In practice, the result of the computation on, say, x86 will depend on
whether the computation is performed using the 80-bit x87 FPU registers or one
of the more modern 64-bit registers, whether extended precision is enabled,
which rounding mode is set, whether FPU 80-bit registers got spilled to 64-bit
stack slots, etc.)

Richard

#5

Ok, thanks. I'll be happy to commit patches along the lines of what Edward proposed.

Howard

#6

I am experimenting with libcxx on FreeBSD and got a lot of asserts on
floating point comparisons, most notably in 'libcxx/test/containers/unord',
e.g.:

Assertion failed: (c.load_factor() == (float)c.size()/c.bucket_count()),
function main, file assign_move.pass.cpp, line 71.

Switching on optimization makes them go away. I'm guessing one of the
values gets promoted from a float to a double somewhere, which makes the
comparison fail.

The question is: do the lvalue and rvalue need to be exactly the same? I
think it would be better to use something like:

assert(fabs(c.load_factor() - (float)c.size()/c.bucket_count())<
FLT_EPSILON);

to sidestep the issue. It also has the benefit of being more implementation
independent?

Before changing this I'd like to understand the issue, just to make sure we
wouldn't be hiding a bug.

load_factor() is computed (in<__hash_table>) as:

_LIBCPP_INLINE_VISIBILITY float load_factor() const _NOEXCEPT
{
size_type __bc = bucket_count(); return __bc != 0 ? (float)size() / __bc : 0.f;
  }

which is precisely how the test is computed too. So I currently do not
understand the cause of the assert.

And just to head off the discussion that one should never compare floating
point with equality: I'm aware of how floating point works. But I also know
that it is deterministic, down to the very last bit.

Not so fast. See C++98[expr]p10 / C++11[expr]p11, "The values of the
&#64258;oating operands and the results of&#64258;oating expressions may be
represented in greater
precision and range than that required by the type". There's no requirement
that the compiler do this in a consistent way, or even do it the same way
every time the same expression is evaluated.

(In practice, the result of the computation on, say, x86 will depend on
whether the computation is performed using the 80-bit x87 FPU registers or one
of the more modern 64-bit registers, whether extended precision is enabled,
which rounding mode is set, whether FPU 80-bit registers got spilled to 64-bit
stack slots, etc.)

Richard

Wow, that's very helpful, thanks. Things get dodgy when working with floats and doubles.

Well, I did some further digging and things got weird:

> cat float.c

#include <stdio.h>
#include <float.h>

int main(void) {

         int a = 4;
         int b = 7;
         float f = (float) a/b;
         double d = (double) a/b;

         printf("%e\n", f - (float)a/b );
         printf("%e\n", d - (float)a/b );
}

> clang float.c && ./a.out
2.554485e-08
0.000000e+00

> clang -O2 float.c && ./a.out
0.000000e+00
-2.554485e-08

> clang float.c -S -o float.S
(.....)
     movl $.L.str, (%ecx)
     movl %eax, -56(%ebp) # 4-byte Spill
     calll printf
(.....)

I guess that indicates you're right. Full asm is attached for the curious. I can't determine if what happens is actually right.

Hope it is helpful to some-one, Ed.

PS. The processor is an Atom330, so not your regular run of the mill....

float.S (1.46 KB)