Skip to content

JIT: Added Sve.LoadVectorNonTemporal/NonFaulting/128AndReplicateToVector APIs - #103392

Merged
kunalspathak merged 2 commits into
dotnet:mainfrom
TIHan:sve-last-load-vector
Jun 14, 2024
Merged

JIT: Added Sve.LoadVectorNonTemporal/NonFaulting/128AndReplicateToVector APIs#103392
kunalspathak merged 2 commits into
dotnet:mainfrom
TIHan:sve-last-load-vector

Conversation

@TIHan

@TIHan TIHan commented Jun 13, 2024

Copy link
Copy Markdown
Contributor

Contributes to #99957

Adds:

  • LoadVectorNonTemporal
  • LoadVectorNonFaulting
  • LoadVector128AndReplicateToVector

@ghost

ghost commented Jun 13, 2024

Copy link
Copy Markdown

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.

1 similar comment
@ghost

ghost commented Jun 13, 2024

Copy link
Copy Markdown

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

Copy link
Copy Markdown
Contributor

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

@TIHan

TIHan commented Jun 13, 2024

Copy link
Copy Markdown
Contributor Author

sve_load_vector_results.txt

@dotnet/arm64-contrib @kunalspathak this is ready. The test results do have failures for NonTemporal and 128AndReplicateToVector because of predicate register support.

@@ -0,0 +1,270 @@
// Licensed to the .NET Foundation under one or more agreements.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this file is doing the same job as SveLoadMaskedUnOpTest.template, which is being used for
Vector<byte> LoadVector(Vector<byte> mask, byte* address) etc.

Your new file is better. Can you combine them both into a single template? (either naming is fine)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Originally I wanted this, but it's a bit more difficult because the other Load APIs take different operand types. Can we do that one in another PR?

@kunalspathak kunalspathak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall looks great. Added few questions and comments about test.

Comment thread src/coreclr/jit/hwintrinsiclistarm64sve.h Outdated
Comment thread src/coreclr/jit/hwintrinsiclistarm64sve.h Outdated
@@ -0,0 +1,270 @@
// Licensed to the .NET Foundation under one or more agreements.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

@kunalspathak kunalspathak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants