ARM64: Fix Round Operation

Math.Round is implemented as a target intrinsic for Arm64.
JIT emitted `frinta` which rounds the ties to away.
As described in MSDN
https://msdn.microsoft.com/en-us/library/system.math.round(v=vs.110).aspx#Round2_Example,
we should round the ties to even.
So, I corrected `frintn` instruction instead for such intrinsic expansion.

I've also identified that `EvalMathFuncUnary` incorrectly folds the round
operation when the argument is constant at the compile time.
The logic itself is not the right semantic as described above in MSDN.
I made a separate helper function which is essentially a duplicate of
classlib. This is not Arm64 specific fix but applies to all other targets.
So, I just fixed it while I'm here.

Commit migrated from https://github.com/dotnet/coreclr/commit/75c97cff492584075b6e13d67a901c69c34ad4f9

diff --git a/src/coreclr/src/jit/codegenarm64.cpp b/src/coreclr/src/jit/codegenarm64.cpp
index f6305b8..1473171 100644 (file)
--- a/src/coreclr/src/jit/codegenarm64.cpp
+++ b/src/coreclr/src/jit/codegenarm64.cpp
@@ -6519,7 +6519,7 @@ CodeGen::genIntrinsic(GenTreePtr treeNode)
 
     case CORINFO_INTRINSIC_Round:
         genConsumeOperands(treeNode->AsOp());
-        getEmitter()->emitInsBinary(INS_frinta, emitTypeSize(treeNode), treeNode, srcNode);
+        getEmitter()->emitInsBinary(INS_frintn, emitTypeSize(treeNode), treeNode, srcNode);
         break;
 
     case CORINFO_INTRINSIC_Sqrt:

diff --git a/src/coreclr/src/jit/utils.cpp b/src/coreclr/src/jit/utils.cpp
index c86d124..f92335e 100644 (file)
--- a/src/coreclr/src/jit/utils.cpp
+++ b/src/coreclr/src/jit/utils.cpp
@@ -1614,3 +1614,29 @@ unsigned __int64 FloatingPointUtils::convertDoubleToUInt64(double d) {
 
     return u64;
 }
+
+// Rounds a double-precision floating-point value to the nearest integer,
+// and rounds midpoint values to the nearest even number.
+// Note this should align with classlib in floatnative.cpp
+// Specializing for x86 using a x87 instruction is optional since
+// this outcome is identical across targets.
+double FloatingPointUtils::round(double d)
+{
+    // If the number has no fractional part do nothing
+    // This shortcut is necessary to workaround precision loss in borderline cases on some platforms
+    if (d == (double)(__int64)d)
+        return d;
+
+    double tempVal = (d + 0.5);
+    //We had a number that was equally close to 2 integers.
+    //We need to return the even one.
+    double flrTempVal = floor(tempVal);
+    if (flrTempVal == tempVal) {
+        if (fmod(tempVal, 2.0) != 0) {
+            flrTempVal -= 1.0;
+        }
+    }
+
+    flrTempVal = _copysign(flrTempVal, d);
+    return flrTempVal;
+}

diff --git a/src/coreclr/src/jit/utils.h b/src/coreclr/src/jit/utils.h
index 7f14163..242a862 100644 (file)
--- a/src/coreclr/src/jit/utils.h
+++ b/src/coreclr/src/jit/utils.h
@@ -538,6 +538,8 @@ public:
     static float convertUInt64ToFloat(unsigned __int64 u64);
 
     static unsigned __int64 convertDoubleToUInt64(double d);
+
+    static double round(double d);
 };
 
 #endif // _UTILS_H_

diff --git a/src/coreclr/src/jit/valuenum.cpp b/src/coreclr/src/jit/valuenum.cpp
index 6471075..f928d60 100644 (file)
--- a/src/coreclr/src/jit/valuenum.cpp
+++ b/src/coreclr/src/jit/valuenum.cpp
@@ -3082,7 +3082,7 @@ ValueNum ValueNumStore::EvalMathFuncUnary(var_types typ, CorInfoIntrinsics gtMat
             res = fabs(arg0Val); // The result and params are doubles.
             break;
         case CORINFO_INTRINSIC_Round:
-            res = (arg0Val > 0.0 ? floor(arg0Val + 0.5) : ceil(arg0Val - 0.5));
+            res = FloatingPointUtils::round(arg0Val);
             break;
         default:
             unreached(); // the above are the only math intrinsics at the time of this writing.

diff --git a/src/coreclr/tests/arm64/Tests.lst b/src/coreclr/tests/arm64/Tests.lst
index 56ff447..fe2acd0 100644 (file)
--- a/src/coreclr/tests/arm64/Tests.lst
+++ b/src/coreclr/tests/arm64/Tests.lst
@@ -46645,7 +46645,7 @@ RelativePath=CoreMangLib\cti\system\convert\ConvertToUInt6413\ConvertToUInt6413.
 WorkingDir=CoreMangLib\cti\system\convert\ConvertToUInt6413
 Expected=0
 MaxAllowedDurationSeconds=600
-Categories=Pri1;RT;UNSTABLE
+Categories=Pri1;RT;EXPECTED_PASS
 HostStyle=0
 [ConvertToUInt6414.cmd_6768]
 RelativePath=CoreMangLib\cti\system\convert\ConvertToUInt6414\ConvertToUInt6414.cmd
@@ -50418,7 +50418,7 @@ RelativePath=CoreMangLib\cti\system\math\MathIEEERemainder\MathIEEERemainder.cmd
 WorkingDir=CoreMangLib\cti\system\math\MathIEEERemainder
 Expected=0
 MaxAllowedDurationSeconds=600
-Categories=Pri1;RT;EXPECTED_FAIL;DBG_PASS;NEED_TRIAGE
+Categories=Pri1;RT;EXPECTED_PASS
 HostStyle=0
 [MathLog.cmd_7307]
 RelativePath=CoreMangLib\cti\system\math\MathLog\MathLog.cmd
@@ -50614,7 +50614,7 @@ RelativePath=CoreMangLib\cti\system\math\MathRound2\MathRound2.cmd
 WorkingDir=CoreMangLib\cti\system\math\MathRound2
 Expected=0
 MaxAllowedDurationSeconds=600
-Categories=Pri1;RT;EXPECTED_FAIL;NEED_TRIAGE;DBG_PASS
+Categories=Pri1;RT;EXPECTED_PASS
 HostStyle=0
 [MathRound3.cmd_7335]
 RelativePath=CoreMangLib\cti\system\math\MathRound3\MathRound3.cmd
@@ -50628,7 +50628,7 @@ RelativePath=CoreMangLib\cti\system\math\MathRound4\MathRound4.cmd
 WorkingDir=CoreMangLib\cti\system\math\MathRound4
 Expected=0
 MaxAllowedDurationSeconds=600
-Categories=Pri1;RT;EXPECTED_FAIL;NEED_TRIAGE;DBG_PASS
+Categories=Pri1;RT;EXPECTED_PASS
 HostStyle=0
 [MathSign1.cmd_7337]
 RelativePath=CoreMangLib\cti\system\math\MathSign1\MathSign1.cmd