Skip to content
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

Jump to bottom

Simplify floating point mod and round jit helpers implementations #100222

Merged
merged 1 commit into from
Mar 26, 2024
Merged

Simplify floating point mod and round jit helpers implementations #100222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 0 additions & 4 deletions src/coreclr/System.Private.CoreLib/src/System/Math.CoreCLR.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,6 @@ public static unsafe (double Sin, double Cos) SinCos(double x)
[MethodImpl(MethodImplOptions.InternalCall)]
public static extern double Tanh(double value);

[Intrinsic]
[MethodImpl(MethodImplOptions.InternalCall)]
private static extern double FMod(double x, double y);

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern unsafe double ModF(double x, double* intptr);

Expand Down
4 changes: 0 additions & 4 deletions src/coreclr/System.Private.CoreLib/src/System/MathF.CoreCLR.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,6 @@ public static unsafe (float Sin, float Cos) SinCos(float x)
[MethodImpl(MethodImplOptions.InternalCall)]
public static extern float Tanh(float x);

[Intrinsic]
[MethodImpl(MethodImplOptions.InternalCall)]
private static extern float FMod(float x, float y);

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern unsafe float ModF(float x, float* intptr);

Expand Down
9 changes: 0 additions & 9 deletions src/coreclr/classlibnative/float/floatdouble.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,15 +181,6 @@ FCIMPLEND
#pragma float_control(pop)
#endif

/*=====================================FMod=====================================
**
==============================================================================*/
FCIMPL2_VV(double, COMDouble::FMod, double x, double y)
FCALL_CONTRACT;

return fmod(x, y);
FCIMPLEND

/*=====================================FusedMultiplyAdd==========================
**
==============================================================================*/
Expand Down
9 changes: 0 additions & 9 deletions src/coreclr/classlibnative/float/floatsingle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,6 @@ FCIMPL1_V(float, COMSingle::Floor, float x)
return floorf(x);
FCIMPLEND

/*=====================================FMod=====================================
**
==============================================================================*/
FCIMPL2_VV(float, COMSingle::FMod, float x, float y)
FCALL_CONTRACT;

return fmodf(x, y);
FCIMPLEND

/*=====================================FusedMultiplyAdd==========================
**
==============================================================================*/
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/classlibnative/inc/floatdouble.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class COMDouble {
FCDECL1_V(static double, Cosh, double x);
FCDECL1_V(static double, Exp, double x);
FCDECL1_V(static double, Floor, double x);
FCDECL2_VV(static double, FMod, double x, double y);
FCDECL3_VVV(static double, FusedMultiplyAdd, double x, double y, double z);
FCDECL1_V(static double, Log, double x);
FCDECL1_V(static double, Log2, double x);
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/classlibnative/inc/floatsingle.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class COMSingle {
FCDECL1_V(static float, Cosh, float x);
FCDECL1_V(static float, Exp, float x);
FCDECL1_V(static float, Floor, float x);
FCDECL2_VV(static float, FMod, float x, float y);
FCDECL3_VVV(static float, FusedMultiplyAdd, float x, float y, float z);
FCDECL1_V(static float, Log, float x);
FCDECL1_V(static float, Log2, float x);
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/inc/corinfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,8 @@ enum CorInfoHelpFunc
CORINFO_HELP_DBL2ULNG_OVF,
CORINFO_HELP_FLTREM,
CORINFO_HELP_DBLREM,
CORINFO_HELP_FLTROUND,
CORINFO_HELP_DBLROUND,
CORINFO_HELP_FLTROUND, // unused, remove once MINIMUM_READYTORUN_MAJOR_VERSION > 9
CORINFO_HELP_DBLROUND, // unused, remove once MINIMUM_READYTORUN_MAJOR_VERSION > 9

/* Allocating a new object. Always use ICorClassInfo::getNewHelper() to decide
which is the right helper to use to allocate an object of a given type. */
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/inc/jithelpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@
JITHELPER(CORINFO_HELP_DBL2ULNG_OVF, JIT_Dbl2ULngOvf, CORINFO_HELP_SIG_8_STACK)
JITHELPER(CORINFO_HELP_FLTREM, JIT_FltRem, CORINFO_HELP_SIG_8_STACK)
JITHELPER(CORINFO_HELP_DBLREM, JIT_DblRem, CORINFO_HELP_SIG_16_STACK)
JITHELPER(CORINFO_HELP_FLTROUND, JIT_FloatRound, CORINFO_HELP_SIG_8_STACK)
JITHELPER(CORINFO_HELP_DBLROUND, JIT_DoubleRound, CORINFO_HELP_SIG_16_STACK)
DYNAMICJITHELPER(CORINFO_HELP_FLTROUND, NULL, CORINFO_HELP_SIG_8_STACK)
DYNAMICJITHELPER(CORINFO_HELP_DBLROUND, NULL, CORINFO_HELP_SIG_16_STACK)

// Allocating a new object
JITHELPER(CORINFO_HELP_NEWFAST, JIT_New, CORINFO_HELP_SIG_REG_ONLY)
Expand Down Expand Up @@ -203,7 +203,7 @@
JITHELPER(CORINFO_HELP_GETSHARED_NONGCTHREADSTATIC_BASE, JIT_GetSharedNonGCThreadStaticBase, CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_GETSHARED_GCTHREADSTATIC_BASE_NOCTOR, JIT_GetSharedGCThreadStaticBase, CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_GETSHARED_NONGCTHREADSTATIC_BASE_NOCTOR, JIT_GetSharedNonGCThreadStaticBase, CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_GETSHARED_GCTHREADSTATIC_BASE_DYNAMICCLASS, JIT_GetSharedGCThreadStaticBaseDynamicClass, CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_GETSHARED_GCTHREADSTATIC_BASE_DYNAMICCLASS, JIT_GetSharedGCThreadStaticBaseDynamicClass, CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_GETSHARED_NONGCTHREADSTATIC_BASE_DYNAMICCLASS, JIT_GetSharedNonGCThreadStaticBaseDynamicClass, CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_GETSHARED_GCTHREADSTATIC_BASE_NOCTOR_OPTIMIZED, JIT_GetSharedGCThreadStaticBaseOptimized, CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_GETSHARED_NONGCTHREADSTATIC_BASE_NOCTOR_OPTIMIZED, JIT_GetSharedNonGCThreadStaticBaseOptimized, CORINFO_HELP_SIG_REG_ONLY)
Expand Down
4 changes: 0 additions & 4 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5627,7 +5627,6 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree)
case NI_System_Math_Cosh:
case NI_System_Math_Exp:
case NI_System_Math_Floor:
case NI_System_Math_FMod:
case NI_System_Math_FusedMultiplyAdd:
case NI_System_Math_ILogB:
case NI_System_Math_Log:
Expand Down Expand Up @@ -12771,9 +12770,6 @@ void Compiler::gtDispTree(GenTree* tree,
case NI_System_Math_Floor:
printf(" floor");
break;
case NI_System_Math_FMod:
printf(" fmod");
break;
case NI_System_Math_FusedMultiplyAdd:
printf(" fma");
break;
Expand Down
6 changes: 0 additions & 6 deletions src/coreclr/jit/importercalls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4033,7 +4033,6 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,
case NI_System_Math_Cosh:
case NI_System_Math_Exp:
case NI_System_Math_Floor:
case NI_System_Math_FMod:
case NI_System_Math_ILogB:
case NI_System_Math_Log:
case NI_System_Math_Log2:
Expand Down Expand Up @@ -7369,7 +7368,6 @@ bool Compiler::IsMathIntrinsic(NamedIntrinsic intrinsicName)
case NI_System_Math_Cosh:
case NI_System_Math_Exp:
case NI_System_Math_Floor:
case NI_System_Math_FMod:
case NI_System_Math_FusedMultiplyAdd:
case NI_System_Math_ILogB:
case NI_System_Math_Log:
Expand Down Expand Up @@ -10084,10 +10082,6 @@ NamedIntrinsic Compiler::lookupPrimitiveFloatNamedIntrinsic(CORINFO_METHOD_HANDL
{
result = NI_System_Math_Floor;
}
else if (strcmp(methodName, "FMod") == 0)
{
result = NI_System_Math_FMod;
}
else if (strcmp(methodName, "FusedMultiplyAdd") == 0)
{
result = NI_System_Math_FusedMultiplyAdd;
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/namedintrinsiclist.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ enum NamedIntrinsic : unsigned short
NI_System_Math_Cosh,
NI_System_Math_Exp,
NI_System_Math_Floor,
NI_System_Math_FMod,
NI_System_Math_FusedMultiplyAdd,
NI_System_Math_ILogB,
NI_System_Math_Log,
Expand Down
20 changes: 0 additions & 20 deletions src/coreclr/jit/valuenum.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8743,14 +8743,6 @@ ValueNum ValueNumStore::EvalMathFuncBinary(var_types typ, NamedIntrinsic gtMathF
break;
}

case NI_System_Math_FMod:
{
assert(typ == TypeOfVN(arg1VN));
double arg1Val = GetConstantDouble(arg1VN);
res = fmod(arg0Val, arg1Val);
break;
}

case NI_System_Math_Pow:
{
assert(typ == TypeOfVN(arg1VN));
Expand Down Expand Up @@ -8848,14 +8840,6 @@ ValueNum ValueNumStore::EvalMathFuncBinary(var_types typ, NamedIntrinsic gtMathF
break;
}

case NI_System_Math_FMod:
{
assert(typ == TypeOfVN(arg1VN));
float arg1Val = GetConstantSingle(arg1VN);
res = fmodf(arg0Val, arg1Val);
break;
}

case NI_System_Math_Max:
{
assert(typ == TypeOfVN(arg1VN));
Expand Down Expand Up @@ -8946,10 +8930,6 @@ ValueNum ValueNumStore::EvalMathFuncBinary(var_types typ, NamedIntrinsic gtMathF
vnf = VNF_Atan2;
break;

case NI_System_Math_FMod:
vnf = VNF_FMod;
break;

case NI_System_Math_Max:
vnf = VNF_Max;
break;
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/valuenumfuncs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ ValueNumFuncDef(Cos, 1, false, false, false, false)
ValueNumFuncDef(Cosh, 1, false, false, false, false)
ValueNumFuncDef(Exp, 1, false, false, false, false)
ValueNumFuncDef(Floor, 1, false, false, false, false)
ValueNumFuncDef(FMod, 2, false, false, false, false)
ValueNumFuncDef(ILogB, 1, false, false, false, false)
ValueNumFuncDef(Log, 1, false, false, false, false)
ValueNumFuncDef(Log2, 1, false, false, false, false)
Expand Down
67 changes: 12 additions & 55 deletions src/coreclr/nativeaot/Runtime/MathHelpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,61 +26,6 @@ FCIMPL1_D(uint64_t, RhpDbl2ULng, double val)
}
FCIMPLEND

#undef min
#undef max
#include <cmath>

FCIMPL2_FF(float, RhpFltRem, float dividend, float divisor)
{
//
// From the ECMA standard:
//
// If [divisor] is zero or [dividend] is infinity
// the result is NaN.
// If [divisor] is infinity,
// the result is [dividend] (negated for -infinity***).
//
// ***"negated for -infinity" has been removed from the spec
//

if (divisor==0 || !std::isfinite(dividend))
{
return -nanf("");
}
else if (!std::isfinite(divisor) && !std::isnan(divisor))
{
return dividend;
}
// else...
return fmodf(dividend,divisor);
}
FCIMPLEND

FCIMPL2_DD(double, RhpDblRem, double dividend, double divisor)
{
//
// From the ECMA standard:
//
// If [divisor] is zero or [dividend] is infinity
// the result is NaN.
// If [divisor] is infinity,
// the result is [dividend] (negated for -infinity***).
//
// ***"negated for -infinity" has been removed from the spec
//
if (divisor==0 || !std::isfinite(dividend))
{
return -nan("");
}
else if (!std::isfinite(divisor) && !std::isnan(divisor))
{
return dividend;
}
// else...
return(fmod(dividend,divisor));
}
FCIMPLEND

#ifndef HOST_64BIT
EXTERN_C int64_t QCALLTYPE RhpLDiv(int64_t i, int64_t j)
{
Expand Down Expand Up @@ -187,6 +132,10 @@ EXTERN_C int64_t F_CALL_CONV RhpLLsh(int64_t i, int32_t j)

#ifdef HOST_X86

#undef min
#undef max
#include <cmath>

FCIMPL1_D(double, acos, double x)
return std::acos(x);
FCIMPLEND
Expand Down Expand Up @@ -363,6 +312,14 @@ FCIMPL1_F(float, tanhf, float x)
return std::tanhf(x);
FCIMPLEND

FCIMPL2_DD(double, fmod, double x, double y)
return std::fmod(x, y);
FCIMPLEND

FCIMPL2_FF(float, fmodf, float x, float y)
return std::fmodf(x, y);
FCIMPLEND

FCIMPL3_DDD(double, fma, double x, double y, double z)
return std::fma(x, y, z);
FCIMPLEND
Expand Down
6 changes: 0 additions & 6 deletions src/coreclr/nativeaot/System.Private.CoreLib/src/System/Math.NativeAot.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,6 @@ public static double Tanh(double value)
return RuntimeImports.tanh(value);
}

[Intrinsic]
private static double FMod(double x, double y)
{
return RuntimeImports.fmod(x, y);
}

[Intrinsic]
private static unsafe double ModF(double x, double* intptr)
{
Expand Down
6 changes: 0 additions & 6 deletions src/coreclr/nativeaot/System.Private.CoreLib/src/System/MathF.NativeAot.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,6 @@ public static float Tanh(float x)
return RuntimeImports.tanhf(x);
}

[Intrinsic]
private static float FMod(float x, float y)
{
return RuntimeImports.fmodf(x, y);
}

[Intrinsic]
private static unsafe float ModF(float x, float* intptr)
{
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,10 @@ public static void GetEntryPoint(TypeSystemContext context, ReadyToRunHelper id,
break;

case ReadyToRunHelper.DblRem:
mangledName = "RhpDblRem";
mangledName = "fmod";
break;
case ReadyToRunHelper.FltRem:
mangledName = "RhpFltRem";
mangledName = "fmodf";
break;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed that DblRound/FltRound are unhandled here (and on main), would it make sense to readd handling for them at least until they're fully removed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what you mean by unhandled. Could you please elaborate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what you mean by unhandled. Could you please elaborate?

There seems to be no switch case for them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
case ReadyToRunHelper.DblRound:
DefType doubleType = context.GetWellKnownType(WellKnownType.Double);
methodDesc = context.SystemModule.GetKnownType("System", "Math").GetKnownMethod("Round",
new MethodSignature(MethodSignatureFlags.Static, 0, doubleType, [doubleType]));
break;
case ReadyToRunHelper.FltRound:
DefType floatType = context.GetWellKnownType(WellKnownType.Single);
methodDesc = context.SystemModule.GetKnownType("System", "MathF").GetKnownMethod("Round",
new MethodSignature(MethodSignatureFlags.Static, 0, floatType, [floatType]));
break;

if we want to add them here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mapping is native AOT specific. It does not need to handle JIT helpers that are no longer used or that are only used outside native AOT.

case ReadyToRunHelper.LMul:
Expand Down
13 changes: 12 additions & 1 deletion src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1224,7 +1224,7 @@ private void ImportBinaryOperation(ILOpcode opcode)
break;
case ILOpcode.mul_ovf:
case ILOpcode.mul_ovf_un:
if (_compilation.TypeSystemContext.Target.Architecture == TargetArchitecture.ARM)
if (_compilation.TypeSystemContext.Target.PointerSize == 4)
{
_dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.LMulOfv), "_lmulovf");
_dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.ULMulOvf), "_ulmulovf");
Expand All @@ -1244,6 +1244,10 @@ private void ImportBinaryOperation(ILOpcode opcode)
else if (_compilation.TypeSystemContext.Target.Architecture == TargetArchitecture.ARM64)
{
_dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.ThrowDivZero), "_divbyzero");
if (opcode == ILOpcode.div)
{
_dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.Overflow), "_ovf");
}
}
break;
case ILOpcode.rem:
Expand All @@ -1258,7 +1262,14 @@ private void ImportBinaryOperation(ILOpcode opcode)
else if (_compilation.TypeSystemContext.Target.Architecture == TargetArchitecture.ARM64)
{
_dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.ThrowDivZero), "_divbyzero");
if (opcode == ILOpcode.rem)
{
_dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.Overflow), "_ovf");
}
}

_dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.DblRem), "rem");
_dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.FltRem), "rem");
jkotas marked this conversation as resolved.
Show resolved Hide resolved
break;
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/vm/corelib.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,12 @@ DEFINE_METHOD(DELEGATE, GET_INVOKE_METHOD, GetInvokeMethod,
DEFINE_CLASS(INT128, System, Int128)
DEFINE_CLASS(UINT128, System, UInt128)

DEFINE_CLASS(MATH, System, Math)
DEFINE_METHOD(MATH, ROUND, Round, SM_Dbl_RetDbl)

DEFINE_CLASS(MATHF, System, MathF)
DEFINE_METHOD(MATHF, ROUND, Round, SM_Flt_RetFlt)

DEFINE_CLASS(DYNAMICMETHOD, ReflectionEmit, DynamicMethod)

DEFINE_CLASS(DYNAMICRESOLVER, ReflectionEmit, DynamicResolver)
Expand Down
8 changes: 8 additions & 0 deletions src/coreclr/vm/ecall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,14 @@ void ECall::PopulateManagedHelpers()
pMD = CoreLibBinder::GetMethod((BinderMethodID)(METHOD__SPAN_HELPERS__MEMCOPY));
pDest = pMD->GetMultiCallableAddrOfCode();
SetJitHelperFunction(CORINFO_HELP_MEMCPY, pDest);

pMD = CoreLibBinder::GetMethod((BinderMethodID)(METHOD__MATH__ROUND));
pDest = pMD->GetMultiCallableAddrOfCode();
SetJitHelperFunction(CORINFO_HELP_DBLROUND, pDest);

pMD = CoreLibBinder::GetMethod((BinderMethodID)(METHOD__MATHF__ROUND));
pDest = pMD->GetMultiCallableAddrOfCode();
SetJitHelperFunction(CORINFO_HELP_FLTROUND, pDest);
}

static CrstStatic gFCallLock;
Expand Down
Loading
Loading