Warning: comparing floating point with == or != is unsafe [-Wfloat-equal] #37

Activity

1. vgrudenic commented on Dec 23, 2018

   

   vgrudenic

   on Dec 23, 2018

   Last edited by vgrudenic

   Contributor

   More actions

   The simplest way would probably be to rewrite it to use something like:

   // presuming diff is positive
   if ( diff > 0.5 ) {
   
   } else if ( !(diff < 0.5) ) {
       // if not > and not <, then it must be ==
       // (but this will also happen if diff is NaN)
   }

   I am not sure how to check for NaN without including <math.h>. So to get an actual comparison, I believe you would have to do something like this:

   bool is_zero_point_five(double num)
   {
       // private union, used to get the
       // bitwise representation of the float
       union double_to_u32
       {
           uint64_t as_uint;
           double as_float;
       };
   
       union double_to_u32 zero_point_five = { .as_float = 0.5 };
       union double_to_u32 value = { .as_float = num };
   
       return value.as_uint == zero_point_five.as_uint;;
   }

   Basically, this uses a union to get an unsigned 64 bit representation of the double, and then compares the integers.

   This gives no warnings and produces rather simple code in gcc. With optimizations enabled, GCC simply compares the value to 0x3FE0000000000000, which is the IEEE-754 representation of 0.5:

   // gcc ARM 64
   is_zero_point_five:
           fmov    x1, d0
           mov     x0, 4602678819172646912
           cmp     x1, x0
           cset    w0, eq
           ret
   
   // gcc x86-64 
   is_zero_point_five:
           movabs  rax, 4602678819172646912
           movq    rdx, xmm0
           cmp     rdx, rax
           sete    al
           ret

   This can be generalized to:

   static inline bool double_compare_exact(double a, double b)
   {
       union double_to_u32 { uint64_t as_uint; double as_float; };
   
       union double_to_u32 aa = { .as_float = a };
       union double_to_u32 bb = { .as_float = b };
   
       return aa.as_uint == bb.as_uint;
   }

   Due to the fact that gcc must account for NaNs and perhaps other floating point exceptions, it cannot optimize !(x < 0.5) && !(x > 0.5) to the equivalent of x != 0.5. However, although the union implementation doesn't produce warnings, it presumes that sizeof(double) is 8 (which should be true on practically all platforms).

   👍React with 👍2Reacted by Marco Paland and Lukas Martini
2. mpaland commented on Dec 23, 2018

   

   mpaland

   on Dec 23, 2018

   Owner

   More actions

   Hello @HarjitSi,

   thanks a lot for bringing this problem to attention.
   I'm aware of this "bug" and I don't like it either. Comparing floats to discrete numbers is never a sure/good thing.
   The whole ftoa routine needs further testing and minor refactoring. This is still on the todo list.
   Anyway, this is just a warning but if you need to be warning free, a construct of greater/smaller comparisons might help - as a quick fix.
3. mpaland commented on Dec 24, 2018

   

   mpaland

   on Dec 24, 2018

   Owner

   More actions

   @vgrudenic , thanks a lot for your suggestion.
   Is double really and always 8 bytes on any platform?
   I have to investigate this a little further...

   Merry christmas! 🎄
4. mpaland

   added

   enhancement

   investigateInvestigation neededInvestigation needed

   on Dec 24, 2018
5. vgrudenic commented on Dec 24, 2018

   

   vgrudenic

   on Dec 24, 2018

   Last edited by vgrudenic

   Contributor

   More actions

   Hi, merry christmas to you too! 🎄😃

   No, I believe the C standard says nothing about the size, although IEEE 754 / IEC 60559 specifies float and double as 32 and 64-bit.

   Perhaps my comment was unnecessarily complicated, and the most reasonable option would be to suppress the warning in GCC? This should leave it to compiler to produce the correct code for a given platform, regardless of the size of floats:

   // doesn't have to be a separate method
   static inline bool double_are_equal(double a, double b)
   {
   
   // temporarily suppress "comparing float with == is unsafe"
   // (we really want to check exactly against 0.5)
   #ifdef __GNUC__
   #pragma GCC diagnostic push
   #pragma GCC diagnostic ignored "-Wfloat-equal"
   #endif
   
       return a == b;
   
   #ifdef __GNUC__
   #pragma GCC diagnostic pop
   #endif
   
   }
6. mpaland

   closed this as completedin 80b42feon Jan 26, 2019