Remove negative number check from float sqrt

[ollie27]

It hasn't been UB to pass negative numbers to sqrt since https://reviews.llvm.org/D28797 which was included in LLVM 5.

[rust-highfive]

r? @LukasKalbertodt

(rust_highfive has picked a reviewer for you, use r? to override)

[Centril]

r? @rkruppe

[Others]

This is backed up by the reference, which defers to IEEE-754, which notes that:

For operations producing results in floating-point format, the default result of an operation that signals the invalid operation exception shall be a quiet NaN that should provide some diagnostic information (see 6.2).
These operations are:
...
g) squareRoot if the operand is less than zero

[hanna-kruppe]

I am reasonably sure we should merge this, but wow, what a mess this situation turned out to be on closer inspection.

There are strange subtleties to what "IEEE compliance" means. During that review and in other related patches, concerns were raised over floating point exceptions caused by sqrt of negative operations. The end result is that e.g. llvm.sqrt(<negative constant>) is not constant folded to a NaN result. This also seems to apply to other intrinsics (e.g. llvm.sin(infinity)).

This bothers me: not only does it mean this PR theoretically removes some optimization power (today, the "redundant" check means (-1.0f32).sqrt() is folded to a NaN regardless), it also seems inconsistent on the part of LLVM. The general policy of LLVM that floating point instructions are assumed to execute in a default floating point environment that makes these concerns moot. While there is no explicit statement one way or another whether this applies to these floating point intrinsics as well, note:

• The constant folding of these intrinsics does in fact blindly assume the default rounding mode and that inexact exceptions don't matter.
• The intrinsics are marked readnone speculateable which makes it legal to reorder them almost arbitrarily w.r.t. other operations (in particular function calls or inline assembly that inspect or change the floating point environment).
• There are corresponding constrained FP intrinsics available that don't have either of these problems and are thus clearly the correct choice for compiling programs that fiddle with the floating point environment but don't care about errno.

So it appears to me that the most consistent and useful interpretation of llvm.{sqrt,sin,...} is that they execute in the default FP environment and can thus be constant folded in the exceptional cases as well. That they aren't constant folded today is likely rooted in historic confusion and inconsistency about how to deal with non-default floating point environments (for reference, constrained FP intrinsics were first added in 2017, and the "default FP environment" concept wasn't introduced until 2018).

I think we should follow up with an LLVM bug to get this apparent inconsistency in LLVM addressed. In the mean time, I think we should merge this PR, as the improvement in the variable argument case probably outweighs the small loss of constant folding. cc @nagisa @nikic for a second opinion.

[nikic]

@rkruppe Agree that this change is fine, and that LLVM is in principle allowed to constant fold these. I think a large part of the reason why that doesn't happen is that constant folding for FP libcalls and intrinsics is largely shared (where the former cannot be folded due to the magic of errno), and that it relies on the host libm (not sure how reliable these tend to be where error cases are concerned).

[hanna-kruppe]

Alright. @bors r+

[bors]

📌 Commit a35b423 has been approved by rkruppe