Change rint to roundeven in round_ties_even implementation

[tgross35]

C has three reasonably similar ways to round a single float to even. I'll just copy the docs here, nearbyint:

Description

The nearbyint functions round their argument to an integer value in floating-point format, using the current rounding direction and without raising the "inexact" floating-point exception.

Returns

The nearbyint functions return the rounded integer value.

rint:

Description

The rint functions differ from the nearbyint functions (7.12.9.3) only in that the rint functions may raise the "inexact" floating-point exception if the result differs in value from the argument

Returns

The rint functions return the rounded integer value.

And roundeven, which is available since C23:

Description

The roundeven functions round their argument to the nearest integer value in floating-point format, rounding halfway cases to even (that is, to the nearest value that is an even integer), regardless of the current rounding direction

Returns

The round functions return the rounded integer value.

We currently use rintf16, rintf32, rintf64, and rintf128 (e.g. https://doc.rust-lang.org/std/intrinsics/fn.rintf32.html) as the intrinsics to implement round_ties_even. These map to LLVM's rint and similar:

rust/compiler/rustc_codegen_llvm/src/intrinsic.rs

Lines 129 to 132 in 613bdd4

	sym::rintf16 => "llvm.rint.f16",
	sym::rintf32 => "llvm.rint.f32",
	sym::rintf64 => "llvm.rint.f64",
	sym::rintf128 => "llvm.rint.f128",

rint is not exactly what we want here because it is supposed to raise inexact. We should change to roundeven because it matches Rust's preference of ignoring rounding modes and never raising fp exceptions.

This may be blocked on platform support, e.g. llvm/llvm-project#73588. that issue looks like it is about the vector version.

Cc #55107.

[tgross35]

I tested the below code on a handful of Tier1/2/3 targets with llc main and didn't run into any selection failures, so LLVM support seems fine. However, on many platforms this lowers to a libcall; since roundint is C23, it is probably safe to say that this isn't available in system libraries on many of our platforms. So I think I need to add this to libm before we make any changes here.

define noundef float @round_f32(float noundef %x) unnamed_addr {
start:
  %0 = tail call float @llvm.round.f32(float %x)
  ret float %0
}

define noundef float @rint_f32(float noundef %x) unnamed_addr {
start:
  %0 = tail call float @llvm.rint.f32(float %x)
  ret float %0
}

define noundef float @roundeven_f32(float noundef %x) unnamed_addr {
start:
  %0 = tail call float @llvm.roundeven.f32(float %x)
  ret float %0
}

define noundef double @round_f64(double noundef %x) unnamed_addr {
start:
  %0 = tail call double @llvm.round.f64(double %x)
  ret double %0
}

define noundef double @rint_f64(double noundef %x) unnamed_addr {
start:
  %0 = tail call double @llvm.rint.f64(double %x)
  ret double %0

}
define noundef double @roundeven_f64(double noundef %x) unnamed_addr {
start:
  %0 = tail call double @llvm.roundeven.f64(double %x)
  ret double %0
}

[hanna-kruppe]

rint is not exactly what we want here because it is supposed to raise inexact. We should change to roundeven because it matches Rust's preference of ignoring rounding modes and never raising fp exceptions.

Keep in mind that the C standard and related documents are written with FENV_ACCESS in mind, but LLVM’s normal (not “constrained”) intrinsics operate in what LLVM calls the default floating point environment, where rounding mode is always to nearest and exceptions are ignored, same as what Rust does. The lowering on most platforms still raises exceptions but that’s because it’s the most efficient lowering and the difference doesn’t matter for programs that adhere to the “default fenv” rules. So I don’t see any reason to change the lowering from Rust to LLVM IR.

[tgross35]

I should have clarified this but the impression I got from other threads is that roundeven is more amenable to optimization than rint. AIUI even though LLVM doesn't have fenv access, it is not allowed to autovectorize rint while it can for exception-free nearbyint https://llvm.org/docs/Vectorizers.html#vectorization-of-function-calls. I am assuming roundeven just hasn't made it onto that list yet because it is newer? (cc @nikic if you know any context). And per #55107 (comment), roundeven may have better hardware support, though I haven't dug into this.

[RalfJung]

FWIW, LLVM also has nearbyint intrinsics. Maybe we should use that?

As far as I can tell, the LLVM semantics of nearbyint, rint, and roundeven are entirely identical when used without "Constrained Floating-Point Intrinsics" metadata, since they all assume the default FP environment -- @nikic does that sound right? So it seems like an LLVM bug to me that they would get vectorized differently.

[hanna-kruppe]

See also llvm/llvm-project#77561

[nikic]

I should have clarified this but the impression I got from other threads is that roundeven is more amenable to optimization than rint. AIUI even though LLVM doesn't have fenv access, it is not allowed to autovectorize rint while it can for exception-free nearbyint https://llvm.org/docs/Vectorizers.html#vectorization-of-function-calls. I am assuming roundeven just hasn't made it onto that list yet because it is newer? (cc @nikic

I think that documentations is just outdated, in reality all three support vectorization: https://github.com/llvm/llvm-project/blob/76e73ae6af1cecffaf977391e52bb9c410c14ff1/llvm/lib/Analysis/VectorUtils.cpp#L94-L97

[tgross35]

FWIW, LLVM also has nearbyint intrinsics. Maybe we should use that?

That would also work. But since C23 is now out I feel as though we may as well just use roundeven, if for no other reason than it being the best representation of our intent. Polyfill in libm would be needed of course.

[hanna-kruppe]

Ideally LLVM would have only one intrinsic for this operation and deprecate/remove (auto-upgrade) the other equivalent ones, so that this confusion doesn’t continue indefinitely. If this happens, Rust should of course be updated accordingly. Otherwise, switching from one equivalent intrinsic to another seems like pointless churn that risks regressions and risks propagating the confusion further.

[hanna-kruppe]

I guess we can just rename the core::intrinsics functions to match the stable Rust name for this operation (round_ties_even) without necessarily changing what LLVM IR it lowers to. Then there would be a single place in the code where a comment can explain the situation.

[tgross35]

I agree that it would be best to only have one intrinsic. But I disagree that changing to a function with obvious implications would somehow make things more confusing; roundeven is obvious across C, LLVM, and Rust. rint acts differently based on context and which of the languages you are looking at.

Also the intrinsics already exist https://doc.rust-lang.org/std/intrinsics/fn.roundevenf32.html, removing rint is a cleanup.

[hanna-kruppe]

Oh dear, why do we have that one too? It doesn’t seem to be used anywhere in-tree. Would be good to clean that up and only have one intrinsic, but I don’t think anything has changed since #95317 (comment) so we probably can’t just replace rint with roundeven. At minimum we’d need a fallback definition of the C symbols that LLVM may try to call in compiler-builtins. That seems like more work than consolidating on a single intrinsic on the Rust side but still lowering it to LLVM’s rint intrinsics.

[RalfJung]

Yeah, having a single Rust intrinsic for this would definitely be a good first cleanup step, so that the decision of how to lower to LLVM can be adjusted entirely locally in the LLVM codegen backend.

[RalfJung]

#136543 cleans up our intrinsics.

Unless we have any evidence that rint is actually a problem, I am not sure if there's anything else we should do? At least, assuming LLVM properly vectorizes rint, I am not sure what the problem being tracked by this issue is. :)

[nikic]

Ideally LLVM would have only one intrinsic for this operation and deprecate/remove (auto-upgrade) the other equivalent ones, so that this confusion doesn’t continue indefinitely. If this happens, Rust should of course be updated accordingly. Otherwise, switching from one equivalent intrinsic to another seems like pointless churn that risks regressions and risks propagating the confusion further.

There are plans to replace the constrained FP intrinsics with operand bundles on normal intrinsics, so I'm not sure this makes sense anymore.

[Jules-Bertholet]

I’m the one who added these intrinsics to Rust, and the stable round_ties_even (#95317). I wanted to use roundeven at first. But Windows libc didn’t support it, so I switched to rint, while keeping the roundeven intrinsics around to make it easier to make the switch in the future.

AFAICT, the only difference between these intrinsics is which C function call LLVM will lower them to, if it does lower them to such a call.