[RISC-V] Flush-to-zero behavior for float-to-int conversion

[denis-paranichev]

RISC-V specification does not provide option which enables flushing to zero return value of float-to-int conversion instructions in the case when input value is NaN or result can't be represented in the target type.
Thus we need to enable this option to support deterministic behavior across platforms.

Part of #84834
cc @gbalykov @t-mustafin @clamp03 @tomeksowi @brucehoult @tannergooding

[ghost]

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

Issue Details

---

RISC-V specification does not provide option which enables flushing to zero return value of float-to-int conversion instructions in the case when input value is NaN or result can't be represented in the target type.
Thus we need to enable this option to support deterministic behavior across platforms.

Part of #84834
cc @gbalykov @t-mustafin @clamp03 @tomeksowi @brucehoult @tannergooding

Author:	DenisParal
Assignees:	-
Labels:	area-CodeGen-coreclr, community-contribution
Milestone:	-

[tannergooding]

which enables flushing to zero return value of float-to-int conversion instructions in the case when input value is NaN or result can't be represented in the target type.

@DenisParal, I think there may have been a slight miscommunication.

The behavior is not "any unrepresentable value becomes 0", but rather the behavior is saturation for finite and infinite values, and with NaN becoming 0 (as there is no other sensible behavior).

[tomeksowi]

the behavior is saturation for finite and infinite values, and with NaN becoming 0

In that case, I believe only the NaN behavior needs mending, this can be done branchless on RV, sth like:

fcvt dest, src       # convert
feq temp, src, src   # NaN check
neg temp, temp       # zero if NaN, all 1s otherwise
and dest, dest, temp # mask the result

[gbalykov]

fcvt dest, src # convert
feq temp, src, src # NaN check
neg temp, temp # zero if NaN, all 1s otherwise
and dest, dest, temp # mask the result

@tomeksowi why is neg needed? feq will set temp to 0 if src is NaN.

[tomeksowi]

@tomeksowi why is neg needed? feq will set temp to 0 if src is NaN.

But it's 1 if not NaN, we need an all 1s mask (-1).

[denis-paranichev]

Fixed flash-to-zero behavior to react only on NaN input.

[denis-paranichev]

@tannergooding, I have tested float.MaxValue conversion to uint on x64 and it seems like this input also causes flushing the result to zero. Is it expected that RISC-V has different result for such input?
Here is reproducer
sample.tar.gz

[tannergooding]

I have tested float.MaxValue conversion to uint on x64 and it seems like this input also causes flushing the result to zero

As mentioned on the other threads that caused this PR, we are working on normalizing all platforms, including x64, to follow the standardized behavior. This work is tracked by #61885

The work to correctly handle x64 is being handled in parallel and will end up matching the stated behavior long term:

• NaN becomes 0
• Saturation occurs for finite and infinite inputs that are out of range

[clamp03]

Please update code formatting with jit-format https://github.com/dotnet/jitutils/blob/main/doc/formatting.md

[tannergooding]

The logic, assuming I understood the RISC-V instructions behavior correctly; looks correct to me.

Someone from @dotnet/jit-contrib needs to review/sign-off on the other changes.

[denis-paranichev]

A lot of System.Tests.UriMethodTests are failed with last commit, I work on fixing it.

[clamp03]

@DenisParal
I found that tmpReg allocated to the same register of treeNode->GetRegNum() in your code.
I implemented code like the below. System.Tests.UriMethodTests passed.
Could you review the codes and tests coreclr tests and corefx tests?
Thank you.

cc @tomeksowi @gbalykov

diff --git a/src/coreclr/jit/lsrariscv64.cpp b/src/coreclr/jit/lsrariscv64.cpp
index 2cf45116227..70396764d30 100644
--- a/src/coreclr/jit/lsrariscv64.cpp
+++ b/src/coreclr/jit/lsrariscv64.cpp
@@ -326,7 +326,9 @@ int LinearScan::BuildNode(GenTree* tree)
 
         case GT_CAST:
             assert(dstCount == 1);
-            buildInternalIntRegisterDefForNode(tree);
             srcCount = BuildCast(tree->AsCast());
-            buildInternalRegisterUses();
             break;
 
         case GT_NEG:
@@ -1269,6 +1271,12 @@ int LinearScan::BuildCast(GenTreeCast* cast)
     int srcCount = BuildOperandUses(cast->CastOp());
     BuildDef(cast);
 
+    if (varTypeIsFloating(cast->gtOp1) && !varTypeIsFloating(cast->TypeGet()))
+    {
+        buildInternalIntRegisterDefForNode(cast);
+        buildInternalRegisterUses();
+    }
+
     return srcCount;
 }

diff --git a/src/coreclr/jit/codegenriscv64.cpp b/src/coreclr/jit/codegenriscv64.cpp
index c2dafce1fb0..d708d4aab5c 100644
--- a/src/coreclr/jit/codegenriscv64.cpp
+++ b/src/coreclr/jit/codegenriscv64.cpp
@@ -3379,9 +3379,26 @@ void CodeGen::genFloatToIntCast(GenTree* treeNode)
     }
 
     genConsumeOperands(treeNode->AsOp());
+    regNumber tmpReg = treeNode->GetSingleTempReg();
+    assert(tmpReg != treeNode->GetRegNum());
+    assert(tmpReg != op1->GetRegNum());
 }

[denis-paranichev]

@clamp03, thank you for suggestion, it looks better than my solution. coreclr tests are passed thus remove draft status.

[denis-paranichev]

@jakobbotsch could you please review this PR?

[jakobbotsch]

Is this a common/official alias?

[denis-paranichev]

It is official according to RISC-V specification.
https://riscv.org/wp-content/uploads/2015/01/riscv-calling.pdf