Skip to content

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.

@ghost
Copy link

ghost commented May 22, 2024

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
Contributor

@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!

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
Contributor

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
Contributor

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
Contributor

/ba-g Issues seems to be #100174

@kunalspathak kunalspathak merged commit 6e52445 into dotnet:main May 23, 2024
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