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: LeadingSignCount, LeadingZeroCount, PopCount #102548

Merged
merged 7 commits into from
May 23, 2024

Conversation

a74nh
Copy link
Contributor

@a74nh a74nh commented May 22, 2024

_SveMasklessUnaryOpTestTemplate is the same as _SveUnaryOpTestTemplate, but without the Conditional Select tests.

For all these, the API method has a different Op1 and Return type and so does not fit into a standard conditional select test.

For LeadingSignCount, LeadingZeroCount the underlying instruction does not have a mask, so can't be optimised using conditional select.

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.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label May 22, 2024
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.

@a74nh a74nh changed the title ARM64-SVE: LeadingSignCount + LeadingZeroCount ARM64-SVE: LeadingSignCount, LeadingZeroCount, PopCount May 22, 2024
@a74nh
Copy link
Contributor Author

a74nh commented May 22, 2024

I get some failures in the stress test due to isLowPredicateRegister() asserts, even though the hwintrinsiclist is correct. I'm assuming this will be fixed by #102543

In the meantime, marking this as ready for review.

@dotnet/arm64-contrib @kunalspathak

@a74nh a74nh marked this pull request as ready for review May 22, 2024 11:22
@kunalspathak kunalspathak added the arm-sve Work related to arm64 SVE/SVE2 support label May 22, 2024
Copy link
Member

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@@ -68,6 +68,8 @@ HARDWARE_INTRINSIC(Sve, FusedMultiplyAddNegated,
HARDWARE_INTRINSIC(Sve, FusedMultiplySubtract, -1, -1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_fmls, INS_sve_fmls}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_EmbeddedMaskedOperation|HW_Flag_HasRMWSemantics|HW_Flag_LowMaskedOperation|HW_Flag_FmaIntrinsic|HW_Flag_SpecialCodeGen)
HARDWARE_INTRINSIC(Sve, FusedMultiplySubtractBySelectedScalar, -1, 4, true, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_fmls, INS_sve_fmls}, HW_Category_SIMDByIndexedElement, HW_Flag_Scalable|HW_Flag_HasImmediateOperand|HW_Flag_HasRMWSemantics|HW_Flag_FmaIntrinsic|HW_Flag_LowVectorOperation)
HARDWARE_INTRINSIC(Sve, FusedMultiplySubtractNegated, -1, -1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_fnmls, INS_sve_fnmls}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_EmbeddedMaskedOperation|HW_Flag_HasRMWSemantics|HW_Flag_LowMaskedOperation|HW_Flag_FmaIntrinsic|HW_Flag_SpecialCodeGen)
HARDWARE_INTRINSIC(Sve, LeadingSignCount, -1, -1, false, {INS_sve_cls, INS_invalid, INS_sve_cls, INS_invalid, INS_sve_cls, INS_invalid, INS_sve_cls, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_BaseTypeFromFirstArg|HW_Flag_EmbeddedMaskedOperation)
Copy link
Member

Choose a reason for hiding this comment

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

these 2 are missing the flag for HW_Flag_LowMaskedOperation

Copy link
Member

Choose a reason for hiding this comment

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

CLS and CLZ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really did think I had added that! Thanks for fixing.

@kunalspathak
Copy link
Member

/ba-g Issues seems to be #100174

@kunalspathak kunalspathak merged commit 6e52445 into dotnet:main May 23, 2024
164 of 167 checks passed
steveharter pushed a commit to steveharter/runtime that referenced this pull request May 28, 2024
* ARM64-SVE: LeadingSignCount + LeadingZeroCount

* Add popcount

* Fix PlatformNotSupported

* Fix summary headers for popcount

* Use SveSimpleVecOpTest for unsigned popcounts

* Add HW_Flag_LowMaskedOperation() to LeadingSignCount() and LeadingZeroCount()

---------

Co-authored-by: Kunal Pathak <[email protected]>
@a74nh a74nh deleted the leadingsign_github branch May 29, 2024 10:18
Ruihan-Yin pushed a commit to Ruihan-Yin/runtime that referenced this pull request May 30, 2024
* ARM64-SVE: LeadingSignCount + LeadingZeroCount

* Add popcount

* Fix PlatformNotSupported

* Fix summary headers for popcount

* Use SveSimpleVecOpTest for unsigned popcounts

* Add HW_Flag_LowMaskedOperation() to LeadingSignCount() and LeadingZeroCount()

---------

Co-authored-by: Kunal Pathak <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Jun 29, 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 community-contribution Indicates that the PR has been added by a community member new-api-needs-documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants