Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Does this work with Scanner? Can the implementation just call Math.Round instead?

I do not understand why we need these JIT intrinsics in the first place. They seem to be only used on Arm and only to morph System.Math.Round. What would break if these JIT intrinsics are deleted?

cc @dotnet/jit-contrib

ARM doesn't have native instructions to perform the rounding (unlike eg. ARM64). I didn't realize there is actually already a managed implementation in Math.cs. I suppose it can be used.

ARM doesn't have native instructions to perform the rounding (unlike eg. ARM64). I didn't realize there is actually already a managed implementation in Math.cs. I suppose it can be used.

Even if there were no managed impl, it could still just go the InternalCall path like most Math APIs.

I wasn't able to track who and why added those, looks like it happened before the Initial commit to populate CoreCLR repo commit.

Even if there were no managed impl, it could still just go the InternalCall path like most Math APIs.

The roundeven native function is not available on all supported platforms. Thus we cannot use it. It was only added in C23 standard, glibc 2.25, and it’s not present on MUSL and Bionic libc.

Even if there were no managed impl, it could still just go the InternalCall path like most Math APIs.

The roundeven native function is not available on all supported platforms. Thus we cannot use it. It was only added in C23 standard, glibc 2.25, and it’s not present on MUSL and Bionic libc.

Does doing

fesetround(FE_TONEAREST);
return rintf(f);

instead not work?

fesetround(FE_TONEAREST); could possibly work too, but if there is existing managed implementation I think it’s easier to just delete intrinsic code path altogether.

I wasn't able to track who and why added those, looks like it happened before the Initial commit to populate CoreCLR repo commit.

It is likely a left over from the times before we had support for recognizing managed methods as JIT intrinsics where all JIT intrinsics had to be FCalls.

Worth noting as well that fesetround(FE_TONEAREST); should be completely unnecessary, as we already do not support the user changing the default rounding mode. It is strictly undefined behavior if they do. nearbyint is notably better than rint, typically, but as we don't support IEEE 754 exception handling and likewise treat enabling it as UB, they should generally behave the same.

However, I also agree that having this in managed is ultimately better. Our currently implementation is notably not super efficient and we could improve it, but that can be a separate work item.

Build break during crossgen: Assertion failed 'unreached' in 'System.Math:IEEERemainder(double,double):double' during 'Linear scan register alloc' (IL size 150; hash 0xdbe0c805; FullOpts)

I think it is also safe to delete the references in jit\valuenum.*, including FltRound/DblRound VNs.

Build break during crossgen: Assertion failed 'unreached' in 'System.Math:IEEERemainder(double,double):double' during 'Linear scan register alloc' (IL size 150; hash 0xdbe0c805; FullOpts)

This will take me a little while to understand and resolve.

@filipnavara I think you need to take out NI_System_Math_Round from here: