Skip to content

Fix Equal and value comparison for double and float types #3088

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 6, 2025
Merged

Fix Equal and value comparison for double and float types #3088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6f26858
Fix `Equal` and value comparison for double and float types
josesimoes Feb 5, 2025
1f0e374
Code style fixes (#143)
nfbot Feb 5, 2025
81564de
Code style fixes (#145)
nfbot Feb 6, 2025
54add5c
Minor code improvements
josesimoes Feb 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 54 additions & 6 deletions src/CLR/Core/CLR_RT_HeapBlock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//
//
// Copyright (c) .NET Foundation and Contributors
// Portions Copyright (c) Microsoft Corporation. All rights reserved.
// See LICENSE file in the project root for full license information.
Expand Down Expand Up @@ -1102,8 +1102,21 @@ CLR_UINT32 CLR_RT_HeapBlock::GetHashCode(CLR_RT_HeapBlock *ptr, bool fRecurse, C
break;

case DATATYPE_R4:
crc = (CLR_INT32)ptr->NumericByRef().u8.LL;
{
// ensure that NaN and both zeros have the same hash code
int signBit = __signbitd(ptr->NumericByRef().r4);

if (__isnanf(ptr->NumericByRef().r4) || (signBit && ptr->NumericByRef().r4 == 0))
{
crc = (CLR_INT32)(ptr->NumericByRef().u8.LL & 0x7FFFFFFF);
}
else
{
crc = (CLR_INT32)ptr->NumericByRef().u8.LL;
}

break;
}

case DATATYPE_U8:
crc = ((CLR_INT32)ptr->NumericByRef().u8.LL ^ (CLR_INT32)ptr->NumericByRef().u8.HH);
Expand All @@ -1114,8 +1127,20 @@ CLR_UINT32 CLR_RT_HeapBlock::GetHashCode(CLR_RT_HeapBlock *ptr, bool fRecurse, C
break;

case DATATYPE_R8:
crc = ((CLR_INT32)ptr->NumericByRef().r8.LL ^ (CLR_INT32)ptr->NumericByRef().r8.HH);
{
// ensure that NaN and both zeros have the same hash code
int signBit = __signbitd((double)ptr->NumericByRef().r8);

if (__isnand((double)ptr->NumericByRef().r8) || (signBit && (double)ptr->NumericByRef().r8 == 0))
{
crc = (CLR_INT32)(ptr->NumericByRef().r8.LL ^ ((CLR_INT32)ptr->NumericByRef().r8.HH & 0x7FFFFFFF));
}
else
{
crc = ((CLR_INT32)ptr->NumericByRef().r8.LL ^ (CLR_INT32)ptr->NumericByRef().r8.HH);
}
break;
}

case DATATYPE_CLASS:
case DATATYPE_VALUETYPE:
Expand Down Expand Up @@ -1336,6 +1361,29 @@ bool CLR_RT_HeapBlock::ObjectsEqual(
return Compare_Values(pArgLeft, pArgRight, false) == 0;
break;

// edge cases, in .NET a NaN is equal to another NaN
// https://learn.microsoft.com/en-us/dotnet/fundamentals/runtime-libraries/system-double-equals?WT.mc_id=DT-MVP-5004179#nan
case DATATYPE_R4:
if (__isnanf(pArgLeft.NumericByRefConst().r4) && __isnanf(pArgRight.NumericByRefConst().r4))
{
return true;
}
else
{
return Compare_Values(pArgLeft, pArgRight, false) == 0;
}
break;
case DATATYPE_R8:
if (__isnand((double)pArgLeft.NumericByRefConst().r8) && __isnand((double)pArgRight.NumericByRefConst().r8))
{
return true;
}
else
{
return Compare_Values(pArgLeft, pArgRight, false) == 0;
}
break;

case DATATYPE_BYREF:
if (rightDataType == DATATYPE_OBJECT)
{
Expand Down Expand Up @@ -1655,9 +1703,9 @@ CLR_INT32 CLR_RT_HeapBlock::Compare_Values(const CLR_RT_HeapBlock &left, const C
case DATATYPE_R4:

// deal with special cases:
// return 0 if the numbers are unordered (either or both are NaN)
// return 1 if the numbers are unordered (either or both are NaN)
// this is post processed in interpreter so '1' will turn into '0'
if (__isnand(left.NumericByRefConst().r4) && __isnand(right.NumericByRefConst().r4))
if (__isnanf(left.NumericByRefConst().r4) || __isnanf(right.NumericByRefConst().r4))
{
return 1;
}
Expand Down Expand Up @@ -1688,7 +1736,7 @@ CLR_INT32 CLR_RT_HeapBlock::Compare_Values(const CLR_RT_HeapBlock &left, const C
case DATATYPE_R8:

// deal with special cases:
// return 0 if the numbers are unordered (either or both are NaN)
// return 1 if the numbers are unordered (either or both are NaN)
// this is post processed in interpreter so '1' will turn into '0'
if (__isnand((double)left.NumericByRefConst().r8) || __isnand((double)right.NumericByRefConst().r8))
{
Expand Down
5 changes: 3 additions & 2 deletions src/PAL/Include/nanoPAL_NativeDouble.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//
//
// Copyright (c) .NET Foundation and Contributors
// Portions Copyright (c) Microsoft Corporation. All rights reserved.
// See LICENSE file in the project root for full license information.
Expand All @@ -14,6 +14,7 @@
#include <float.h>

#define __isnand _isnan
#define __isnanf _isnan
#define __isinfd(x) (!_finite(x))

inline int __signbitd(double x)
Expand All @@ -23,7 +24,7 @@ inline int __signbitd(double x)
}

#define rint(x) floor((x) + 0.5)
#define remainder(x, y) ((x) - ((y)*rint((x) / (y))))
#define remainder(x, y) ((x) - ((y) * rint((x) / (y))))

#define isgreater(param0, param1) (param0 > param1)
#define isless(param0, param1) (param0 < param1)
Expand Down