1
\$\begingroup\$

I'm currently rolling a little Float16 implementation. It's minimalistic and can't handle infinity, NaN and will neither work properly with overflows nor with underflows. (That's not the goal btw.)

Is there something particular wrong with the following implementation which could lead to major issues?

class Float16
{
public:
    Float16(float other)
    {
        IEEESingle f32;
        f32.Float = other;

        if (other == 0.0f)
        {
            bits.IEEE.Exp = 0;
            bits.IEEE.Man = 0;

            return;
        }

        bits.IEEE.Sign = f32.IEEE.Sign;
        bits.IEEE.Exp  = f32.IEEE.Exp - 112;
        bits.IEEE.Man  = f32.IEEE.Man >> 13;
    }

    operator float() const
    {
        if (bits.IEEE.Exp == 0 && bits.IEEE.Man == 0)
        {
            return 0.0f;
        }

        IEEESingle f32;

        //112-->F32 Exponent = N + 127, F16 Exponent = N + 15; 

        f32.IEEE.Sign = bits.IEEE.Sign;
        f32.IEEE.Exp  = bits.IEEE.Exp + 112; 
        f32.IEEE.Man  = bits.IEEE.Man << 13;

        //<< 13 since Mantisse f32 = 23 and Mantisse F16 = 10

        return f32.Float;
    }

private:
    union IEEEHalf
    {
        uint16_t bits;
        struct
        {
            uint16_t Man  : 10;
            uint16_t Exp  : 5;
            uint16_t Sign : 1;
        } IEEE;
    } bits;

    union IEEESingle
    {
        float Float;
        struct
        {
            uint32_t Man  : 23;
            uint32_t Exp  : 8;
            uint32_t Sign : 1;
        } IEEE;
    };
};

int main()
{
    Float16 f16 = 1.5f;
    float   f32 = (float)f16;

    return 0;
}

I am aware that my calculations will happen at F32 level and I don't intend to implement direct F16 calculations. I am planning to add some operators to the class, but the question focuses mostly on the conversion between f16 and f32. I am aware of the precision and rounding issues of Float16 values.

\$\endgroup\$
  • \$\begingroup\$ Use static assert and std::numerical_limits to make sure that float is defined in the way that you assume. Note there is no requirement in the standard for float to be a IEEESingle. \$\endgroup\$ – Martin York Apr 23 '18 at 18:36
  • \$\begingroup\$ Read access to a union member that was not the last member assigned to is UB in C++. \$\endgroup\$ – Martin York Apr 23 '18 at 18:37
  • \$\begingroup\$ Are you doing this just as an exercise? \$\endgroup\$ – Martin York Apr 23 '18 at 18:40
  • \$\begingroup\$ @Martin York I want to try some things with FP16 and shaders. On the target system I can rely on float to be IEEE Single. I could feed the GPU with F32 values but that would ne half the fun. ;) \$\endgroup\$ – Mango Apr 23 '18 at 19:08
  • \$\begingroup\$ You should still put the static asserts in. Then in the future after you have moved on to more interesting things the code will still work and correctly not compile when the new guy/girl tries to build it on a different system. \$\endgroup\$ – Martin York Apr 23 '18 at 19:32
1
\$\begingroup\$

Is there something particular wrong with the following implementation which could lead to major issues?

Yes; the union seems to be used in a way that is actually Undefined Behavior. Let’s see…

    f32.IEEE.Sign = bits.IEEE.Sign;
    f32.IEEE.Exp  = bits.IEEE.Exp + 112; 
    f32.IEEE.Man  = bits.IEEE.Man << 13;

    return f32.Float;

Yes, you are reading from the Float member that is never written to, the optimizer can know that it is not the active member, and act as if the program never reaches this point. Anything is possible, including crashing.

We have seen the optimizers get more and more aggressive, taking advantage of U.B., breaking “bad” code that worked historically. So, it is a real concern. In any case, the layout of the bit fields is up to the implementation; that is only an issue if you plan on saving these 16-bit values to files.

See Scott Schurr’s presentation on this. Your specific case is handled toward the end, and he shows that of all the work-arounds, using intrinsic memcpy with the items known to be properly aligned will be completely optimized out by the compiler, generating the code you really intended with the union!


See also ⧺ES.9.


float   f32 = (float)f16;

Don’t use C-style casts. Any appearance of such a cast is a flag for code review; casts that are actually needed should be function style, specialized, or use clearly named helper functions.

You supplied an implicit conversion, so you don’t need this cast at all.

\$\endgroup\$
0
\$\begingroup\$

In C++ writing one branch of a union and then reading from another branch is generally undefined (though often defined the same way it works in C).

\$\endgroup\$
  • \$\begingroup\$ reinterpret_cast is equally undefined behavior. std::memcpy is the only way to get it to be implementation defined behavior, and even that is not that useful. \$\endgroup\$ – nwp Apr 24 '18 at 12:33
  • \$\begingroup\$ I was thinking of composition of reinterpret_casts to char array and back, but you're right, this is equally undefined. \$\endgroup\$ – bipll Apr 24 '18 at 12:43
  • \$\begingroup\$ Needless to say that shifting floatingpoint bits is that plattform dependend that UB is one of my least problems. A change in the float representation format or the float length (what Martin York pointes out) is a much bigger issue. But since I am aware of that, I will simply add the numeric_limits check (even if I don't plan to pass the code around) and leave the UB as UB. I hope that the short float proposal will make it into C++ anytime soon anyway. \$\endgroup\$ – Mango Apr 24 '18 at 12:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.