IEEE 754 defines the expected result on overflow. As far as I know, hardware implementations (of f16), and compiler-rt (__floatuntisf) correctly return +-Inf on overflow. And I can't think of any useful transform that would take advantage of overflow being undefined here.
This is reversing part of D5603 / rL219542 . I see there was some controversy in the post-commit thread, although it was for the FP -> int direction. Let me add those commenters as reviewers in any case.
For reference, IEEE754, section 7.4 says:
"The overflow exception shall be signaled if and only if the destination format’s largest finite number is exceeded in magnitude by what would have been the rounded floating-point result (see 4) were the exponent range unbounded. The default result shall be determined by the rounding-direction attribute and the sign of the intermediate result as follows:
a) roundTiesToEven and roundTiesToAway carry all overflows to ∞ with the sign of the intermediate result."
And the C standard, section 22.214.171.124 says:
"When a value of integer type is converted to a real floating type, if the value being converted can be represented exactly in the new type, it is unchanged. If the value being converted is in the range of values that can be represented but cannot be represented exactly, the result is either the nearest higher or nearest lower representable value, chosen in an implementation-defined manner. If the value being converted is outside the range of values that can be represented, the behavior is undefined."
That language (like the LangRef regardless of what we decide here) probably needs refinement because there is no "outside of the range" of infinity?
Note: I moved/corrected the tests at rL334107 (shouldn't have been under instcombine in the first place). Please rebase with full context for the record.
I agree that this is a good change, so LGTM. But we should wait a bit for more comments in case anyone else sees potential downside.
|3289 ↗||(On Diff #150181)|
Nit - I'd find it easier to read if we rewrote as:
``uitofp (CONSTANT to TYPE)`` Convert an unsigned integer constant to floating-point. CONSTANT must be a scalar or vector integer type. TYPE must be a scalar or vector floating-point type. If CONSTANT is a vector, TYPE must be a vector with the same number of elements.
Rust currently generates additional code for u128 to f32 casts to work around this problem (see https://github.com/rust-lang/rust/blob/c131bdcaff68d35f96e954baac4340206779335f/src/librustc_codegen_llvm/mir/rvalue.rs#L795). Making this defined would allow directly using uitofp, so this would be a welcome change.