Skip to content

Arm AdvSimd: Rename LoadVector128xN to LoadNxVector128 #103626

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

Merged
merged 4 commits into from
Jun 18, 2024

Conversation

a74nh
Copy link
Contributor

@a74nh a74nh commented Jun 18, 2024

Partially fixes #102340

@ghost
Copy link

ghost commented Jun 18, 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.

1 similar comment
@ghost
Copy link

ghost commented Jun 18, 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 Jun 18, 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 force-pushed the LoadNxVector128_github branch from b07ebdb to a0af045 Compare June 18, 2024 10:48
@a74nh
Copy link
Contributor Author

a74nh commented Jun 18, 2024

Manually confirmed HardwareIntrinsics_Arm_ro.dll tests still run for Load2xVector, Load3xVector, Load4xVector.

This is ready now. The mono parts haven't been tested as I don't know that code or how to test it, so needs a review from the mono team.

@kunalspathak @dotnet/arm64-contrib

@a74nh a74nh marked this pull request as ready for review June 18, 2024 13:21
@a74nh a74nh requested a review from fanyang-mono as a code owner June 18, 2024 13:21
Copy link
Member

@fanyang-mono fanyang-mono left a comment

Choose a reason for hiding this comment

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

The change to Mono looks good to me.

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.

coreclr changes LGTM. Thank you for doing this at short notice @a74nh!

@kunalspathak
Copy link
Member

@fanyang-mono - which CI pipelines we should run to make sure that the code in the PR is getting tested for mono?

@fanyang-mono
Copy link
Member

@kunalspathak There isn't a CI line currently could validate this change. I could run the tests locally.

@kunalspathak
Copy link
Member

@kunalspathak There isn't a CI line currently could validate this change. I could run the tests locally.

sounds good. Please confirm here once you do that and I will go ahead and merge it.

@fanyang-mono
Copy link
Member

Tests belonging to AdvSimd.Arm64_r.dll passed.

Supported ISAs:
  AdvSimd:   True
  Aes:       True
  ArmBase:   True
  Crc32:     True
  Dp:        False
  Rdm:       False
  Sha1:      True
  Sha256:    True
  Sve:       False
  
  ...
  
  Expected: 100
Actual: 100
END EXECUTION - PASSED

@kunalspathak
Copy link
Member

/ba-g timeout issues

@kunalspathak kunalspathak merged commit 2fec267 into dotnet:main Jun 18, 2024
154 of 177 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime.Intrinsics 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.

Arm64: Rename AdvSimd/Sve Load/Store vector API names
3 participants