Float to integer conversion may result in undefined behavior

[aykevl]

The Go spec says this about conversions:

In all non-constant conversions involving floating-point or complex values, if the result type cannot represent the value the conversion succeeds but the result value is implementation-dependent.

In other words, all conversions (including float-to-integer) must be implementation defined and not undefined. At the moment, we use LLVM fptui and fptosi instructions, which have this to say on the matter:

The ‘fptoui’ instruction converts its floating-point operand into the nearest (rounding towards zero) unsigned integer value. If the value cannot fit in ty2, the result is a poison value.

The ‘fptosi’ instruction converts its floating-point operand into the nearest (rounding towards zero) signed integer value. If the value cannot fit in ty2, the result is a poison value.

This basically means that floating point values that cannot be represented as integers (Infinity, NaN, negative numbers in unsigned integers) will produce an arbitrary value that if used may invoke undefined behavior.

This should be fixed. It is probably easiest to wait until https://reviews.llvm.org/D54749 is merged to avoid bloating the code too much.

Similar issue in Rust: rust-lang/rust#10184

[niaow]

I think maybe we should insert clamping checks for now since that still has not been merged and rust seems to have decided that adding checks was appropriate.

[aykevl]

Yes that sounds reasonable. Float isn't used much anyway and often it's a softfloat implementation (so relatively slow). I think it's reasonable to add checks and look into optimizations later.

[niaow]

For now I am going to use a select instruction to restrict the output. Once LLVM 12 lands, we can probably use this: https://reviews.llvm.org/rGa89d751fb401540c89189e7c17ff64a6eca98587

[deadprogram]

This has been released with v0.17.0 so now closing. Thanks!

[aykevl]

Once we drop support for LLVM 11, we should start using the LLVM intrinsics instead of doing this manually.
See #2300 for progress on LLVM 12 support.