Add Scalar Intel hardware intrinsic functions

[tannergooding]

This proposal extends #23057 to additionally cover intrinsics for the scalar forms of the x86 hardware instructions.

Rationale

Currently, CoreFX implicitly emits various SSE/SSE2 instructions when performing operations on scalar floating-point types. However, there is currently no way to explicitly emit specific optimized sequences of code to best take advantage of the underlying hardware.

For example, the System.Math and System.MathF APIs are currently implemented as FCALLs to the underlying C Runtime implementation of the corresponding method. The underlying C Runtime implementations are most frequently implemented as hand-optimized assembly or C/C++ intrinsics to ensure that they provide the best throughput possible and that they take advantage of newer instruction sets when available (cos/cosf, for example, frequently have one code path for SSE and one for FMA3).

Due to these methods depending on the underlying C Runtime implementation:

• We are at a significantly delayed cadence for getting bug fixes
  • We often have to add hacks to workaround these bugs as we find them, decreasing perf
• The implementations between platforms and architectures often differ
  • This leads to perf differences: https://github.com/dotnet/coreclr/issues/9373
  • This leads to input/output differences between Operating Systems (Windows, Linux, Mac, etc)
  • This leads to input/output differences between Platforms (x86, x64, ARM, etc)
• Updating these methods requires modifying the runtime

By providing scalar intrinsics for the Intel hardware functions it becomes much easier to implement these functions in managed code, which:

• Means fewer workarounds as bugs can be fixed directly
• Keeps perf is more consistent
• Keeps input/output differences minimal or non-existing between operating systems
• Helps keep input/output are minimal between platforms
• Allows most bug fixes to be made independent of the runtime

Furthermore, with the addition of #23057, it may become more pertinent to also have these scalar intrinsics to ensure that the codegen when intermixing scalar and vectorized operations remains "optimal" for the end-users customized/hand-optimized algorithms.

Proposed API

The current design in #23057 creates a class per instruction set and exposes methods such as Vector128<double> Sse2.Sqrt(Vector128<double>) which corresponds to the __m128d _mm_sqrt_pd(__m128d) C/C++ intrinsic`.

This would additionally extend the surface area to expose the scalar forms of all the instructions with the same name as the vector intrinsic, but with a Scalar postfix. This is required to differentiate between the vector and scalar APIs due to them taking the same types as their inputs.

For example, we would expose:

public static class Sse2
{
    // __m128d _mm_add_sd(__m128d a, __m128d b);
    public static Vector128<double> AddScalar<double>(Vector128<double> left, Vector128<double> right);

    // ...

    // No corresponding C/C++ intrinsic, used when upper should be taken from `value`
    public static Vector128<double> SqrtScalar(Vector128<double> value);    

    // __m128d _mm_sqrt_sd(__m128d a, __m128d b)
    public static Vector128<double> SqrtScalar(Vector128<double> upper, Vector128<double> value);

    // ...
}

Other Thoughts

Most of the remaining sections (Intended Audience, Semantics and Usage, Implementation Roadmap, etc) are the same as in #23057

[tannergooding]

cc: @russellhadley @mellinoe @CarolEidt @terrajobst, @fiigii

[jkotas]

What does Vector128<double> SqrtScalar(Vector128<double> value) corresponds to?

[tannergooding]

SqrtScalar => _mm_sqrt_sd (updated above): https://msdn.microsoft.com/en-us/library/1994h1ay(v=vs.100).aspx

AddScalar => _mm_add_sd (updated above): https://msdn.microsoft.com/en-us/library/we4dxyk3(v=vs.100).aspx

[fiigii]

@tannergooding Thank you for opening this issue. Exposing these SSE* scalar floating-point intrinsics has different concerns from #23057, so discussing this in a new proposal is a good idea.
Do you want all the SSE* scalar floating-point instructions (most of them are already covered current RyuJIT codegen) or just a subset to be exposed as intrinsics?

[tannergooding]

@fiigii, I think they should all be exposed since that will likely lead to the best codegen and reduce pressure on the JIT for the scenarios where these matter. It will also likely lead to less confusion for the end-user when dealing with scalar types as they won't have to remember to convert to Vector128<float> to perform operations 'A', 'B', and 'C', and then convert back to float to perform operation 'X', 'Y', and 'Z'.

I believe the primary use case will be to ensure better codegen occurs when mixing scalar and vectorized operations (I believe this is the purpose behind certain functions such as _mm_mask_sqrt_sd).

But it will also be important for implement optimized forms of various mathematical algorithms in managed code (such as the System.Math, System.MathF, and System.Numerics.Complex APIs).

[fanoI]

What is the purpose of:
cs // __m128d _mm_sqrt_sd(__m128d a, __m128d b) public static Vector128<double> SqrtScalar(Vector128<double> upper, Vector128<double> value);

I mean why it takes 2 parameters? SQRTSD takes only one...

[tannergooding]

@fanol. SQRTSD takes two parameters. The first is the register which receives the result in its lower bits and has its upper bits left untouched. The second is a register or memory address on which the operation is actually performed.

The API needs to expose the ability to dictate what the upper bits of the computed result should be, in the case that it is important. Taking a second parameter allows and places the burden of optimization on the compiler (the other option would be to return nothing and have a ref parameter, which forces the user to allocate a temp themselves if they want to preserve a value).

[tannergooding]

Going to ping @russellhadley @mellinoe @CarolEidt @terrajobst, on this.

The API surface area isn't fully described (since it covers several hundred APIs), but it follows the same convention as the previous proposal so it should be "reviewable".

[CarolEidt]

I'm fine with this in theory, but we need to ensure that we measure throughput as we expand the number of intrinsics that we support.

[tannergooding]

we need to ensure that we measure throughput as we expand the number of intrinsics that we support.

I agree.

With the sheer number of APIs we are doing, when an intrinsic is hit in the System.Runtime.Intrinsics namespace, there are going to end up being a number (possibly in the hundreds) of string comparisons to find the matching method (at least with my limited understanding of the NamedIntrinsics feature).

I think we'll end up needing to find an alternative way to match these intrinsics quickly, without doing a bunch of string comparisons. SSE2 has 144 unique instructions (at least if my reference is correct -- this is both scalar and vector forms), but there are additional overloads on top of that as well.

[4creators]

AFAIR the proposal by @fiigii was to use binary search to get it to be acceptable fast

[CarolEidt]

AFAIR the proposal by @fiigii was to use binary search to get it to be acceptable fast

That may be sufficient, but I still think it is going to be critical to measure each step along the way, so that we know where we stand.

[tannergooding]

@CarolEidt, at one point (before the Intel proposal went up), I had proposed that this be achieved via [Intrinsic(IntrinsicId id)], where IntrinsicId was an enum that had a strict 1-to-1 mapping with an enum in the runtime (what is currently the NamedIntrinsic enum).

Outside of any cost required to get the value out of the attribute for a given method, it would be a constant time lookup, regardless of how many intrinsics we decide to add now or in the future.

[CarolEidt]

@tannergooding - that might be a reasonable solution; it has all the earmarks of fragility (the two sides getting out of sync), but given that the name can be verified, certainly at least in a Checked/Debug build, it could be practical. I've always been told that attribute recognition is expensive in the runtime, except for those attributes that have specific metadata representations, so it would be interesting to hear from someone on the VM side (@jkotas?)

[tannergooding]

I've always been told that attribute recognition is expensive in the runtime, except for those attributes that have specific metadata representations

I think that [Intrinsic] is already in that category (specific metadata representation), since it is what is being used by the NamedIntrinsics today, just that it doesn't take any parameters.

it has all the earmarks of fragility (the two sides getting out of sync), but given that the name can be verified, certainly at least in a Checked/Debug build, it could be practical

I think the general case is, if managed code uses an IntrinsicId that the runtime doesn't recognize, we will get a PNSE, but that was already the case today (if the runtime doesn't know to match the namespace/method/class name).

The "worst" case is that someone ends up changing the enum values in one or the other, such that they no longer line up, but that should be detectable in checked/debug mode (as you indicated). We can also help prevent it from happening by explicitly assigning values to the enum entries and adding warnings at the top/bottom of the enum declarations, to help ensure users see the requirement of keeping them in sync.