odin-lang / Odin

Odin Programming Language
https://odin-lang.org
BSD 3-Clause "New" or "Revised" License
6.55k stars 570 forks source link

Different results when casting a u32 constant and a u32 variable holding the same value to f32 #103

Closed vassvik closed 5 years ago

vassvik commented 6 years ago

The following snippet:

import "core:fmt.odin";

main :: proc() {
    // R :: 268435448; // incorrect
    // R :: 300000000; // correct
    R :: 4294967195; // incorrect
    r := u32(R);
    fmt.println(r == u32(R));
    fmt.println(f32(r) == f32(R));
}

will output

true
false

It does not seem to be only large values, as you can find in-between values where it works as expected.

It seems to be related to floating point rounding:

    R :: 4294967195; // breaks
    r := u32(R);
    f1 := f32(r);
    f2 := f32(R);
    fmt.printf("%.3f %.3f\n", f1, f2);

which will output:

294967296.000 4294967040.000

which seems to indicate that the former rounds up, while the latter rounds down.

dotbmp commented 6 years ago

Just checking in to confirm that both the rounding bug and the fmt bug are still happening.

Sorry 😛

gingerBill commented 6 years ago

This appears to be an LLVM issue with regards to encoding 32 bit floating point numbers: https://github.com/odin-lang/Odin/blob/master/src/ir_print.cpp#L621

u64 u = bit_cast<u64>(value.value_float);
switch (type->Basic.kind) {
case Basic_f32:
    // IMPORTANT NOTE(bill): LLVM requires all floating point constants to be
    // a 64 bit number if bits_of(float type) <= 64.
    // https://groups.google.com/forum/#!topic/llvm-dev/IlqV3TbSk6M
    // 64 bit mantissa: 52 bits
    // 32 bit mantissa: 23 bits
    // 16 bit mantissa: 10 bits
    // 29 == 52-23
    u >>= 29;
    u <<= 29;
    break;
// case Basic_f16:
    // u >>= 42;
    // u <<= 42;
    // break;
}
Kelimion commented 5 years ago

Confirmed to still be broken:

$ odin run main.odin 
true
false
294967296.000 4294967040.000
gingerBill commented 5 years ago

This is a difficult one as I am not sure how to fix it without loads of hacks. This is a design fault of LLVM.

thebirk commented 5 years ago

Just re-ran the example code and this appears to be working when using version 8.0.1 of LLVM binaries.

gingerBill commented 5 years ago

@thebirk I fixed the problem by doing an explicit bit cast from an integer to a float instead of trying to represent the float correctly. LLVM is stupid.