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: Fix Comment group headers in SVE API (#104846) #105679

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

a74nh
Copy link
Contributor

@a74nh a74nh commented Jul 30, 2024

Also fix up Scatter summary blocks.

This was mostly done via regex.

Personally, I'd prefer of the form // AddSaturate : Saturating add, (ie add the method name to the start of the comment). But happy enough with how it is in the PR.

There are still quite a few additional changes that need making to these files to tidy them up, but I'll put them in separate PRs to simplify the review process.

Fixes #104846

Also fix up Scatter summary blocks.
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 30, 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
Copy link
Contributor Author

a74nh commented Jul 30, 2024

@dotnet/arm64-contrib @kunalspathak

Copy link
Member

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

/// <summary>
/// void svst1_scatter_[s32]offset[_s32](svbool_t pg, int32_t *base, svint32_t offsets, svint32_t data)
/// ST1W Zdata.S, Pg, [Xbase, Zoffsets.S, SXTW]
/// </summary>
public static unsafe void Scatter(Vector<int> mask, int* address, Vector<int> indicies, Vector<int> data) { throw new PlatformNotSupportedException(); }

// <summary>
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the APIs that are commented out for now didn't get updated to ///. I'm alright leaving these as-is for now, just pointing this out for future work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AIUI, The ones that are commented out should remain as // because then the summary doesn't end up in the docs.

@amanasifkhalid
Copy link
Member

@a74nh do you have permission to merge your own PRs? If not, I'm happy to do that for you.

@a74nh
Copy link
Contributor Author

a74nh commented Jul 30, 2024

@a74nh do you have permission to merge your own PRs? If not, I'm happy to do that for you.

I got some permissions recently, but not that as far as I can tell. If you could merge for me that'd be great, thanks!

@amanasifkhalid amanasifkhalid merged commit eebac58 into dotnet:main Jul 30, 2024
143 of 147 checks passed
@a74nh a74nh deleted the summary_github branch July 30, 2024 16:16
@github-actions github-actions bot locked and limited conversation to collaborators Aug 30, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JIT: SVE Cleanup - Remove unneeded comments from API surface area
2 participants