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.

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

Change the ReciprocalEstimate and ReciprocalSqrtEstimate APIs to be mustExpand on RyuJIT #102098

Merged
merged 17 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
b2d559e
Change the ReciprocalEstimate and ReciprocalSqrtEstimate APIs to be m…
tannergooding May 10, 2024
30dc925
Apply formatting patch
tannergooding May 10, 2024
67a391b
Fix the RV64 and LA64 builds
tannergooding May 10, 2024
42a1dd5
Mark the ReciprocalEstimate and ReciprocalSqrtEstimate methods as Agg…
tannergooding May 11, 2024
f177bd2
Mark other usages of ReciprocalEstimate and ReciprocalSqrtEstimate in…
tannergooding May 11, 2024
62933bc
Mark several non-deterministic APIs as BypassReadyToRun and skip intr…
tannergooding May 11, 2024
afdcb1b
Cleanup based on PR recommendations to rely on the runtime rather tha…
tannergooding May 11, 2024
91d105c
Adding a regression test ensuring direct and indirect invocation of n…
tannergooding May 11, 2024
ed406bf
Add a note about non-deterministic intrinsic expansion to the botr
tannergooding May 11, 2024
1b16a09
Apply formatting patch
tannergooding May 11, 2024
1000066
Merge branch 'main' into fix-101731
tannergooding May 12, 2024
0312112
Ensure vector tests are correctly validating against the scalar imple…
tannergooding May 12, 2024
461c4b5
Fix the JIT/SIMD/VectorConvert test and workaround a 32-bit test issue
tannergooding May 12, 2024
9983d9d
Skip a test on Mono due to a known/tracked issue
tannergooding May 12, 2024
5bddfc1
Ensure that lowering on Arm64 doesn't make an assumption about cast s…
tannergooding May 12, 2024
1e0879f
Ensure the tier0opts local is used
tannergooding May 13, 2024
f63c575
Ensure impEstimateIntrinsic bails out for APIs that need to be implem…
tannergooding May 13, 2024
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
45 changes: 28 additions & 17 deletions src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3125,15 +3125,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,
// To be fixed in https://github.com/dotnet/runtime/pull/77465
const bool tier0opts = !opts.compDbgCode && !opts.jitFlags->IsSet(JitFlags::JIT_FLAG_MIN_OPT);

if (tier0opts)
{
// The *Estimate APIs are allowed to differ in behavior across hardware
// so ensure we treat them as "betterToExpand" to get deterministic behavior

betterToExpand |= (ni == NI_System_Math_ReciprocalEstimate);
betterToExpand |= (ni == NI_System_Math_ReciprocalSqrtEstimate);
}
else if (!mustExpand)
if (!mustExpand)
{
switch (ni)
{
Expand Down Expand Up @@ -7471,6 +7463,8 @@ bool Compiler::IsTargetIntrinsic(NamedIntrinsic intrinsicName)
{
case NI_System_Math_Abs:
case NI_System_Math_Sqrt:
case NI_System_Math_ReciprocalEstimate:
case NI_System_Math_ReciprocalSqrtEstimate:
return true;

default:
Expand Down Expand Up @@ -8847,7 +8841,7 @@ GenTree* Compiler::impEstimateIntrinsic(CORINFO_METHOD_HANDLE method,
simdSize = 16;
}

GenTree* op1 = impPopStack().val;
GenTree* op1 = impImplicitR4orR8Cast(impPopStack().val, callType);

op1 = gtNewSimdCreateScalarUnsafeNode(simdType, op1, callJitType, simdSize);
op1 = gtNewSimdHWIntrinsicNode(simdType, op1, intrinsicId, callJitType, simdSize);
Expand All @@ -8856,10 +8850,27 @@ GenTree* Compiler::impEstimateIntrinsic(CORINFO_METHOD_HANDLE method,
}
#endif // FEATURE_HW_INTRINSICS

// TODO-CQ: Returning this as an intrinsic blocks inlining and is undesirable
// return impMathIntrinsic(method, sig, callType, intrinsicName, tailCall);
switch (intrinsicName)
{
case NI_System_Math_ReciprocalEstimate:
case NI_System_Math_ReciprocalSqrtEstimate:
{
GenTree* op1 = impImplicitR4orR8Cast(impPopStack().val, callType);

return nullptr;
if (intrinsicName == NI_System_Math_ReciprocalSqrtEstimate)
{
op1 = new (this, GT_INTRINSIC)
GenTreeIntrinsic(genActualType(callType), op1, NI_System_Math_Sqrt, nullptr);
}

return gtNewOperNode(GT_DIV, genActualType(callType), gtNewDconNode(1.0, callType), op1);
}

default:
{
unreached();
}
}
}

GenTree* Compiler::impMathIntrinsic(CORINFO_METHOD_HANDLE method,
Expand Down Expand Up @@ -8958,8 +8969,8 @@ GenTree* Compiler::impMinMaxIntrinsic(CORINFO_METHOD_HANDLE method,
GenTreeDblCon* cnsNode = nullptr;
GenTree* otherNode = nullptr;

GenTree* op2 = impStackTop().val;
GenTree* op1 = impStackTop(1).val;
GenTree* op2 = impImplicitR4orR8Cast(impStackTop().val, callType);
GenTree* op1 = impImplicitR4orR8Cast(impStackTop(1).val, callType);

if (op2->IsCnsFltOrDbl())
{
Expand Down Expand Up @@ -9227,7 +9238,7 @@ GenTree* Compiler::impMinMaxIntrinsic(CORINFO_METHOD_HANDLE method,
retNode->AsHWIntrinsic()->Op(2) = op1;
}

return gtNewSimdToScalarNode(callType, retNode, callJitType, 16);
return gtNewSimdToScalarNode(genActualType(callType), retNode, callJitType, 16);
}
}
#endif // FEATURE_HW_INTRINSICS && TARGET_XARCH
Expand Down Expand Up @@ -9392,7 +9403,7 @@ GenTree* Compiler::impMinMaxIntrinsic(CORINFO_METHOD_HANDLE method,
callJitType, 16);
}

return gtNewSimdToScalarNode(callType, tmp, callJitType, 16);
return gtNewSimdToScalarNode(genActualType(callType), tmp, callJitType, 16);
}
#endif // FEATURE_HW_INTRINSICS && TARGET_XARCH

Expand Down
2 changes: 2 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/Double.cs
Original file line number Diff line number Diff line change
Expand Up @@ -866,10 +866,12 @@ bool IFloatingPoint<double>.TryWriteSignificandLittleEndian(Span<byte> destinati

/// <inheritdoc cref="IFloatingPointIeee754{TSelf}.ReciprocalEstimate(TSelf)" />
[Intrinsic]
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
public static double ReciprocalEstimate(double x) => Math.ReciprocalEstimate(x);

/// <inheritdoc cref="IFloatingPointIeee754{TSelf}.ReciprocalSqrtEstimate(TSelf)" />
[Intrinsic]
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
public static double ReciprocalSqrtEstimate(double x) => Math.ReciprocalSqrtEstimate(x);

/// <inheritdoc cref="IFloatingPointIeee754{TSelf}.ScaleB(TSelf, int)" />
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/Half.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1578,9 +1578,11 @@ public static int ILogB(Half x)
public static Half Lerp(Half value1, Half value2, Half amount) => (Half)float.Lerp((float)value1, (float)value2, (float)amount);

/// <inheritdoc cref="IFloatingPointIeee754{TSelf}.ReciprocalEstimate(TSelf)" />
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
public static Half ReciprocalEstimate(Half x) => (Half)MathF.ReciprocalEstimate((float)x);

/// <inheritdoc cref="IFloatingPointIeee754{TSelf}.ReciprocalSqrtEstimate(TSelf)" />
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
public static Half ReciprocalSqrtEstimate(Half x) => (Half)MathF.ReciprocalSqrtEstimate((float)x);

/// <inheritdoc cref="IFloatingPointIeee754{TSelf}.ScaleB(TSelf, int)" />
Expand Down
22 changes: 12 additions & 10 deletions src/libraries/System.Private.CoreLib/src/System/Math.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1195,12 +1195,13 @@ public static double MinMagnitude(double x, double y)
/// <para>On ARM64 hardware this may use the <c>FRECPE</c> instruction which performs a single Newton-Raphson iteration.</para>
/// <para>On hardware without specialized support, this may just return <c>1.0 / d</c>.</para>
/// </remarks>
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
#if MONO
public static double ReciprocalEstimate(double d) => 1.0 / d;
#else
[Intrinsic]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static double ReciprocalEstimate(double d)
{
return 1.0 / d;
}
public static double ReciprocalEstimate(double d) => ReciprocalEstimate(d);
#endif

/// <summary>Returns an estimate of the reciprocal square root of a specified number.</summary>
/// <param name="d">The number whose reciprocal square root is to be estimated.</param>
Expand All @@ -1209,12 +1210,13 @@ public static double ReciprocalEstimate(double d)
/// <para>On ARM64 hardware this may use the <c>FRSQRTE</c> instruction which performs a single Newton-Raphson iteration.</para>
/// <para>On hardware without specialized support, this may just return <c>1.0 / Sqrt(d)</c>.</para>
/// </remarks>
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
#if MONO || TARGET_RISCV64 || TARGET_LOONGARCH64
public static double ReciprocalSqrtEstimate(double d) => 1.0 / Sqrt(d);
#else
[Intrinsic]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static double ReciprocalSqrtEstimate(double d)
{
return 1.0 / Sqrt(d);
}
public static double ReciprocalSqrtEstimate(double d) => ReciprocalSqrtEstimate(d);
#endif

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static decimal Round(decimal d)
Expand Down
22 changes: 12 additions & 10 deletions src/libraries/System.Private.CoreLib/src/System/MathF.cs
Original file line number Diff line number Diff line change
Expand Up @@ -313,12 +313,13 @@ public static float MinMagnitude(float x, float y)
/// <para>On ARM64 hardware this may use the <c>FRECPE</c> instruction which performs a single Newton-Raphson iteration.</para>
/// <para>On hardware without specialized support, this may just return <c>1.0 / x</c>.</para>
/// </remarks>
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
#if MONO
public static float ReciprocalEstimate(float x) => 1.0f / x;
#else
[Intrinsic]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static float ReciprocalEstimate(float x)
{
return 1.0f / x;
}
public static float ReciprocalEstimate(float x) => ReciprocalEstimate(x);
#endif

/// <summary>Returns an estimate of the reciprocal square root of a specified number.</summary>
/// <param name="x">The number whose reciprocal square root is to be estimated.</param>
Expand All @@ -328,12 +329,13 @@ public static float ReciprocalEstimate(float x)
/// <para>On ARM64 hardware this may use the <c>FRSQRTE</c> instruction which performs a single Newton-Raphson iteration.</para>
/// <para>On hardware without specialized support, this may just return <c>1.0 / Sqrt(x)</c>.</para>
/// </remarks>
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
#if MONO || TARGET_RISCV64 || TARGET_LOONGARCH64
public static float ReciprocalSqrtEstimate(float x) => 1.0f / Sqrt(x);
#else
[Intrinsic]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static float ReciprocalSqrtEstimate(float x)
{
return 1.0f / Sqrt(x);
}
public static float ReciprocalSqrtEstimate(float x) => ReciprocalSqrtEstimate(x);
#endif

[Intrinsic]
public static float Round(float x)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1190,9 +1190,11 @@ bool IFloatingPoint<NFloat>.TryWriteSignificandLittleEndian(Span<byte> destinati
public static NFloat Lerp(NFloat value1, NFloat value2, NFloat amount) => new NFloat(NativeType.Lerp(value1._value, value2._value, amount._value));

/// <inheritdoc cref="IFloatingPointIeee754{TSelf}.ReciprocalEstimate(TSelf)" />
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
public static NFloat ReciprocalEstimate(NFloat x) => new NFloat(NativeType.ReciprocalEstimate(x._value));

/// <inheritdoc cref="IFloatingPointIeee754{TSelf}.ReciprocalSqrtEstimate(TSelf)" />
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
public static NFloat ReciprocalSqrtEstimate(NFloat x) => new NFloat(NativeType.ReciprocalSqrtEstimate(x._value));

/// <inheritdoc cref="IFloatingPointIeee754{TSelf}.ScaleB(TSelf, int)" />
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/Single.cs
Original file line number Diff line number Diff line change
Expand Up @@ -849,10 +849,12 @@ bool IFloatingPoint<float>.TryWriteSignificandLittleEndian(Span<byte> destinatio

/// <inheritdoc cref="IFloatingPointIeee754{TSelf}.ReciprocalEstimate(TSelf)" />
[Intrinsic]
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
public static float ReciprocalEstimate(float x) => MathF.ReciprocalEstimate(x);

/// <inheritdoc cref="IFloatingPointIeee754{TSelf}.ReciprocalSqrtEstimate(TSelf)" />
[Intrinsic]
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
public static float ReciprocalSqrtEstimate(float x) => MathF.ReciprocalSqrtEstimate(x);

/// <inheritdoc cref="IFloatingPointIeee754{TSelf}.ScaleB(TSelf, int)" />
Expand Down
Loading