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

Arm64/Sve: Implement ConditionalSelect API #100718

Closed
wants to merge 4 commits into from

Conversation

kunalspathak
Copy link
Member

image

Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@kunalspathak
Copy link
Member Author

@dotnet/arm64-contrib

Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-runtime-intrinsics
See info in area-owners.md if you want to be subscribed.

@@ -0,0 +1,362 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this template based on an existing template?

Can we just use a generic 3 op template here? I don't think I see anthing specific to conditionalselect


/// <summary>
Copy link
Member

Choose a reason for hiding this comment

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

nit: It's preferred to order these "alphabetically" as well, since that's what tooling will do in various places.

This is done based on the type name, not the language keyword:

  • byte (Byte), double (Double), short (Int16), int (Int32), long (Int64), nint (IntPtr), sbyte (SByte), float (Single), ushort (UInt16), uint (UInt32), ulong (UInt64), nuint (UIntPtr)

Copy link
Member Author

Choose a reason for hiding this comment

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

@a74nh - do you mind fixing the tool to generate these alphabetically?

Copy link
Contributor

Choose a reason for hiding this comment

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

@a74nh - do you mind fixing the tool to generate these alphabetically?

Done. The branch with the autogenerated files should now be in order for all the .cs files.

@@ -4149,7 +4149,16 @@ public new abstract partial class Arm64 : System.Runtime.Intrinsics.Arm.AdvSimd.
public static System.Numerics.Vector<ushort> CreateTrueMaskUInt16([ConstantExpected] SveMaskPattern pattern = SveMaskPattern.All) { throw null; }
public static System.Numerics.Vector<uint> CreateTrueMaskUInt32([ConstantExpected] SveMaskPattern pattern = SveMaskPattern.All) { throw null; }
public static System.Numerics.Vector<ulong> CreateTrueMaskUInt64([ConstantExpected] SveMaskPattern pattern = SveMaskPattern.All) { throw null; }

public static System.Numerics.Vector<sbyte> ConditionalSelect(System.Numerics.Vector<sbyte> mask, System.Numerics.Vector<sbyte> left, System.Numerics.Vector<sbyte> right) { throw null; }
Copy link
Member

Choose a reason for hiding this comment

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

If this is ever generated by the tooling, it's going to change all this to be done alphabetically, hence the comment above.

@kunalspathak kunalspathak added arm-sve Work related to arm64 SVE/SVE2 support NO-REVIEW Experimental/testing PR, do NOT review it labels Apr 12, 2024
@kunalspathak
Copy link
Member Author

Replaced with #100743

@github-actions github-actions bot locked and limited conversation to collaborators May 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime.Intrinsics arm-sve Work related to arm64 SVE/SVE2 support new-api-needs-documentation NO-REVIEW Experimental/testing PR, do NOT review it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants