Runtime: API: Sse2.StoreLow long/ulong overloads should be renamed to Sse2.StoreScalar

Created on 9 Apr 2019  路  11Comments  路  Source: dotnet/runtime

__Proposed API modifications:__

namespace System.Runtime.Intrinsics.X86
{
    public static class Sse2
    {
        // method renamed from "StoreLow" to "StoreScalar"
        public unsafe static void StoreScalar(long* address, Vector128<long> source);

        // method renamed from "StoreLow" to "StoreScalar"
        public unsafe static void StoreScalar(ulong* address, Vector128<ulong> source);
    }
}

These API suggestions follow the existing parameter naming conventions for StoreScalar(double* address, Vector128<double> source). See also Intel Intrinsics Guide.

__Old issue:__

/cc @tannergooding, who originally identified this offline yesterday.

Apparently the movq instruction works just fine in 32-bit mode. We should audit the existing APIs on the _X64_ intrinsic types and see if they're also applicable to 32-bit mode, and if they are then move them down one layer.

api-approved area-System.Runtime.Intrinsics

Most helpful comment

:shipit:

All 11 comments

I don't think we can move these directly. However, we could likely expose void ConvertToInt64(long* address, Vector128<float> value) overloads that store directly to memory.

We should audit the existing APIs on the X64 intrinsic types and see if they're also applicable to 32-bit mode

ConvertToInt64 is a bit special and is the only one I know of in this category. It actually has two separate encodings. The one we are exposing in X64 is the movd/movq overload that can operate on reg/mem, reg. There is a separate standalone movq instruction available in 32-bit mode that is mem, reg exclusive and that is what we could expose on the Sse2 class.

CC. @CarolEidt

@GrabYourPitchforks; could you update the original post to make this an API proposal matching the signature I gave above (and including any other ConvertToInt64 overloads)?

@tannergooding and I spoke offline. Will update the proposal.

@GrabYourPitchforks, I think it is just the StoreScalar overloads that are missing. We are exposing the LoadScalarVector128 ones already: https://source.dot.net/#System.Private.CoreLib/shared/System/Runtime/Intrinsics/X86/Sse2.cs,799

And, actually. It looks like we already have the StoreScalar versions as well: https://source.dot.net/#System.Private.CoreLib/shared/System/Runtime/Intrinsics/X86/Sse2.cs,1453; but they are called StoreLow

Given that we do expose the API; but it is called StoreLow (and there is no equivalent StoreHigh), it should probably be renamed to StoreScalar.... (this would also make it match the LoadScalarVector128 corresponding API).

Thanks! I think this looks good/correct now.

@dotnet/fxdc: @tannergooding would lik to check in this week to hit preview 5. Any concerns? To me, this rename sounds good.

:shipit:

I think we've got quorum to approve this 馃槃

Was this page helpful?
0 / 5 - 0 ratings