Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 1cd5d1b

Browse files
committed
Merge pull request #3655 from sejongoh/fix_u64_to_f64_cast
Fix inconsistent uint64-to-double cast
2 parents 6278721 + 38d980d commit 1cd5d1b

File tree

8 files changed

+204
-26
lines changed

8 files changed

+204
-26
lines changed

src/jit/codegenxarch.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7502,6 +7502,10 @@ CodeGen::genIntToFloatCast(GenTreePtr treeNode)
75027502
// result if sign-bit of srcType is set.
75037503
if (srcType == TYP_ULONG)
75047504
{
7505+
// The instruction sequence below is less accurate than what clang
7506+
// and gcc generate. However, we keep the current sequence for backward compatiblity.
7507+
// If we change the instructions below, FloatingPointUtils::convertUInt64ToDobule
7508+
// should be also updated for consistent conversion result.
75057509
assert(dstType == TYP_DOUBLE);
75067510
assert(!op1->isContained());
75077511

src/jit/gentree.cpp

100644100755
Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9954,7 +9954,7 @@ GenTreePtr Compiler::gtFoldExprConst(GenTreePtr tree)
99549954
case TYP_DOUBLE:
99559955
if ((tree->gtFlags & GTF_UNSIGNED) && lval1 < 0)
99569956
{
9957-
d1 = (double) (unsigned __int64) lval1;
9957+
d1 = FloatingPointUtils::convertUInt64ToDouble((unsigned __int64)lval1);
99589958
}
99599959
else
99609960
{
@@ -10074,29 +10074,8 @@ GenTreePtr Compiler::gtFoldExprConst(GenTreePtr tree)
1007410074
lval1 = INT64(d1); goto CNS_LONG;
1007510075

1007610076
case TYP_ULONG:
10077-
if (d1 >= 0.0)
10078-
{
10079-
// Work around a C++ issue where it doesn't properly convert large positive doubles
10080-
const double two63 = 2147483648.0 * 4294967296.0;
10081-
if (d1 < two63) {
10082-
lval1 = UINT64(d1);
10083-
}
10084-
else {
10085-
// subtract 0x8000000000000000, do the convert then add it back again
10086-
lval1 = INT64(d1 - two63) + I64(0x8000000000000000);
10087-
}
10088-
goto CNS_LONG;
10089-
}
10090-
10091-
// This double cast to account for an ECMA spec hole.
10092-
// When converting from a double to an unsigned the ECMA
10093-
// spec states that a conforming implementation should
10094-
// "truncate to zero." However that doesn't make much sense
10095-
// when the double in question is negative and the target
10096-
// is unsigned. gcc converts a negative double to zero when
10097-
// cast to an unsigned. To make gcc conform to MSVC behavior
10098-
// this cast is necessary.
10099-
lval1 = UINT64(INT64(d1)); goto CNS_LONG;
10077+
lval1 = FloatingPointUtils::convertDoubleToUInt64(d1);
10078+
goto CNS_LONG;
1010010079

1010110080
case TYP_FLOAT:
1010210081
d1 = forceCastToFloat(d1);

src/jit/utils.cpp

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1544,3 +1544,66 @@ unsigned CountDigits(unsigned num, unsigned base /* = 10 */)
15441544
}
15451545

15461546
#endif // DEBUG
1547+
1548+
1549+
double FloatingPointUtils::convertUInt64ToDouble(unsigned __int64 uIntVal) {
1550+
__int64 s64 = uIntVal;
1551+
double d;
1552+
if (s64 < 0) {
1553+
#if defined(_TARGET_XARCH_)
1554+
// RyuJIT codegen and clang (or gcc) may produce different results for casting uint64 to
1555+
// double, and the clang result is more accurate. For example,
1556+
// 1) (double)0x84595161401484A0UL --> 43e08b2a2c280290 (RyuJIT codegen or VC++)
1557+
// 2) (double)0x84595161401484A0UL --> 43e08b2a2c280291 (clang or gcc)
1558+
// If the folding optimization below is implemented by simple casting of (double)uint64_val
1559+
// and it is compiled by clang, casting result can be inconsistent, depending on whether
1560+
// the folding optimization is triggered or the codegen generates instructions for casting. //
1561+
// The current solution is to force the same math as the codegen does, so that casting
1562+
// result is always consistent.
1563+
1564+
// d = (double)(int64_t)uint64 + 0x1p64
1565+
uint64_t adjHex = 0x43F0000000000000UL;
1566+
d = (double)s64 + *(double*)&adjHex;
1567+
#else
1568+
d = (double)uIntVal;
1569+
#endif
1570+
}
1571+
else
1572+
{
1573+
d = (double)uIntVal;
1574+
}
1575+
return d;
1576+
}
1577+
1578+
float FloatingPointUtils::convertUInt64ToFloat(unsigned __int64 u64) {
1579+
double d = convertUInt64ToDouble(u64);
1580+
return (float)d;
1581+
}
1582+
1583+
unsigned __int64 FloatingPointUtils::convertDoubleToUInt64(double d) {
1584+
unsigned __int64 u64;
1585+
if (d >= 0.0)
1586+
{
1587+
// Work around a C++ issue where it doesn't properly convert large positive doubles
1588+
const double two63 = 2147483648.0 * 4294967296.0;
1589+
if (d < two63) {
1590+
u64 = UINT64(d);
1591+
}
1592+
else {
1593+
// subtract 0x8000000000000000, do the convert then add it back again
1594+
u64 = INT64(d - two63) + I64(0x8000000000000000);
1595+
}
1596+
return u64;
1597+
}
1598+
1599+
// This double cast to account for an ECMA spec hole.
1600+
// When converting from a double to an unsigned the ECMA
1601+
// spec states that a conforming implementation should
1602+
// "truncate to zero." However that doesn't make much sense
1603+
// when the double in question is negative and the target
1604+
// is unsigned. gcc converts a negative double to zero when
1605+
// cast to an unsigned. To make gcc conform to MSVC behavior
1606+
// this cast is necessary.
1607+
u64 = UINT64(INT64(d));
1608+
return u64;
1609+
}

src/jit/utils.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,4 +510,17 @@ struct ListNode
510510
}
511511
};
512512

513+
/*****************************************************************************
514+
* Floating point utility class
515+
*/
516+
class FloatingPointUtils {
517+
public:
518+
519+
static double convertUInt64ToDouble(unsigned __int64 u64);
520+
521+
static float convertUInt64ToFloat(unsigned __int64 u64);
522+
523+
static unsigned __int64 convertDoubleToUInt64(double d);
524+
};
525+
513526
#endif // _UTILS_H_

src/jit/valuenum.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1906,13 +1906,13 @@ ValueNum ValueNumStore::EvalCastForConstantArgs(var_types typ, VNFunc func, Valu
19061906
case TYP_FLOAT:
19071907
assert(typ == TYP_FLOAT);
19081908
if (srcIsUnsigned)
1909-
return VNForFloatCon(float(UINT64(arg0Val)));
1909+
return VNForFloatCon(FloatingPointUtils::convertUInt64ToFloat(UINT64(arg0Val)));
19101910
else
19111911
return VNForFloatCon(float(arg0Val));
19121912
case TYP_DOUBLE:
19131913
assert(typ == TYP_DOUBLE);
19141914
if (srcIsUnsigned)
1915-
return VNForDoubleCon(double(UINT64(arg0Val)));
1915+
return VNForDoubleCon(FloatingPointUtils::convertUInt64ToDouble(UINT64(arg0Val)));
19161916
else
19171917
return VNForDoubleCon(double(arg0Val));
19181918
default:
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information
4+
5+
using System;
6+
7+
public class Program
8+
{
9+
// RyuJIT codegen and clang (or gcc) may produce different results for casting uint64 to
10+
// double, and the clang result is more accurate. For example,
11+
// 1) (double)0x84595161401484A0UL --> 43e08b2a2c280290 (RyuJIT codegen or VC++)
12+
// 2) (double)0x84595161401484A0UL --> 43e08b2a2c280291 (clang or gcc)
13+
// Constant folding in RyuJIT simply does (double)0x84595161401484A0UL in its C++ implementation.
14+
// If it is compiled by clang, the example unsigned value and cast tree node are folded into
15+
// 43e08b2a2c280291, which is different from what the codegen produces. To fix this inconsistency,
16+
// the constant folding is forced to have the same behavior as the codegen, and the result
17+
// must be always 43e08b2a2c280290.
18+
public static int Main(string[] args)
19+
{
20+
//Check if the test is being executed on ARMARCH
21+
bool isProcessorArmArch = false;
22+
string processorArchEnvVar = null;
23+
processorArchEnvVar = Environment.GetEnvironmentVariable("PROCESSOR_ARCHITECTURE");
24+
25+
if ((processorArchEnvVar != null)
26+
&& (processorArchEnvVar.Equals("ARM", StringComparison.CurrentCultureIgnoreCase)
27+
|| processorArchEnvVar.Equals("ARM64", StringComparison.CurrentCultureIgnoreCase)))
28+
{
29+
isProcessorArmArch = true;
30+
}
31+
32+
ulong u64 = 0x84595161401484A0UL;
33+
double f64 = (double)u64;
34+
long h64 = BitConverter.DoubleToInt64Bits(f64);
35+
long expected_h64 = isProcessorArmArch ? 0x43e08b2a2c280291L : 0x43e08b2a2c280290L;
36+
if (h64 != expected_h64) {
37+
Console.WriteLine(String.Format("Expected: 0x{0:x}\nActual: 0x{1:x}", expected_h64, h64));
38+
return -1;
39+
}
40+
return 100;
41+
}
42+
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
3+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
4+
<PropertyGroup>
5+
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
6+
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
7+
<AssemblyName>$(MSBuildProjectName)</AssemblyName>
8+
<SchemaVersion>2.0</SchemaVersion>
9+
<ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
10+
<OutputType>Exe</OutputType>
11+
<AppDesignerFolder>Properties</AppDesignerFolder>
12+
<FileAlignment>512</FileAlignment>
13+
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
14+
<ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages</ReferencePath>
15+
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
16+
17+
<NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
18+
</PropertyGroup>
19+
<!-- Default configurations to help VS understand the configurations -->
20+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
21+
</PropertyGroup>
22+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
23+
</PropertyGroup>
24+
<ItemGroup>
25+
<CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
26+
<Visible>False</Visible>
27+
</CodeAnalysisDependentAssemblyPaths>
28+
</ItemGroup>
29+
<PropertyGroup>
30+
<DebugType></DebugType>
31+
<Optimize>True</Optimize>
32+
</PropertyGroup>
33+
<ItemGroup>
34+
<Compile Include="$(MSBuildProjectName).cs" />
35+
</ItemGroup>
36+
<ItemGroup>
37+
<None Include="$(JitPackagesConfigFileDirectory)minimal\project.json" />
38+
<None Include="app.config" />
39+
</ItemGroup>
40+
<ItemGroup>
41+
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
42+
</ItemGroup>
43+
<PropertyGroup>
44+
<ProjectJson>$(JitPackagesConfigFileDirectory)minimal\project.json</ProjectJson>
45+
<ProjectLockJson>$(JitPackagesConfigFileDirectory)minimal\project.lock.json</ProjectLockJson>
46+
</PropertyGroup>
47+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
48+
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
49+
</PropertyGroup>
50+
</Project>
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<configuration>
3+
<runtime>
4+
<assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
5+
<dependentAssembly>
6+
<assemblyIdentity name="System.Runtime" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
7+
<bindingRedirect oldVersion="0.0.0.0-4.0.20.0" newVersion="4.0.20.0" />
8+
</dependentAssembly>
9+
<dependentAssembly>
10+
<assemblyIdentity name="System.Text.Encoding" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
11+
<bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
12+
</dependentAssembly>
13+
<dependentAssembly>
14+
<assemblyIdentity name="System.Threading.Tasks" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
15+
<bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
16+
</dependentAssembly>
17+
<dependentAssembly>
18+
<assemblyIdentity name="System.IO" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
19+
<bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
20+
</dependentAssembly>
21+
<dependentAssembly>
22+
<assemblyIdentity name="System.Reflection" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
23+
<bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
24+
</dependentAssembly>
25+
</assemblyBinding>
26+
</runtime>
27+
</configuration>

0 commit comments

Comments
 (0)