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: Add SignExtend* and ZeroExtend* math APIs #101702

Merged
merged 4 commits into from Apr 30, 2024

Conversation

kunalspathak
Copy link
Member

@kunalspathak kunalspathak commented Apr 29, 2024

  • SignExtend16
  • SignExtend32
  • SignExtend8
  • ZeroExtend16
  • ZeroExtend32
  • ZeroExtend8

All tests passing: https://gist.github.com/kunalspathak/d8c244f02f51e1a9e247f97f9d23b9e3

Contributes to #99957

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.

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.

@kunalspathak kunalspathak added the arm-sve Work related to arm64 SVE/SVE2 support label Apr 29, 2024
@kunalspathak kunalspathak requested a review from TIHan April 29, 2024 23:32
@kunalspathak
Copy link
Member Author

@dotnet/arm64-contrib

Copy link
Member

@TIHan TIHan left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -448,14 +448,13 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
if (intrin.op3->isContained())
{
assert(intrin.op3->IsVectorZero());
if (intrin.op1->isContained())
if (intrin.op1->isContained() || intrin.op1->IsMaskAllBitsSet())
Copy link
Member

Choose a reason for hiding this comment

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

The intrin.op1->isContained() seems pointless here, no?

We just need to know its not contained for an instruction that actually requires its use (like abs)

Copy link
Member Author

Choose a reason for hiding this comment

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

i will have to double check on this one. There was lot of back and forth on this for certain scenarios. I will verify it on next round.

matouskozak pushed a commit to matouskozak/runtime that referenced this pull request Apr 30, 2024
* Add [Sign|Zero]Extend[8|16|32] APIs:

* Add API to instruction mapping

* eliminate extra movprfx for AllBitsSetMask

* Add test cases
michaelgsharp pushed a commit to michaelgsharp/runtime that referenced this pull request May 9, 2024
* Add [Sign|Zero]Extend[8|16|32] APIs:

* Add API to instruction mapping

* eliminate extra movprfx for AllBitsSetMask

* Add test cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants