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: Detect mask usage for Across functions #101973

Closed
a74nh opened this issue May 7, 2024 · 4 comments
Closed

ARM64-SVE: Detect mask usage for Across functions #101973

a74nh opened this issue May 7, 2024 · 4 comments
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI arm-sve Work related to arm64 SVE/SVE2 support in-pr There is an active PR which will close this issue when it is merged Priority:2 Work that is important, but not critical for the release
Milestone

Comments

@a74nh
Copy link
Contributor

a74nh commented May 7, 2024

ConditionalSelect is used to mask an API. For methods that reduce down to a scalar value (eg all the Across methods), this would be done via:

a = Sve.AddAcross(Sve.ConditionalSelect(mask, a, zero));

Currently this will produce an SEL instruction to select the correct entries. Then will feed this into an
SADDV (or UADDV) that uses an all-true mask.

Instead, the SEL needs merging into the SADDV to produce a single SADDV instruction using the mask.

See #101770 for history.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 7, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label May 7, 2024
@jeffschwMSFT jeffschwMSFT added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI arm-sve Work related to arm64 SVE/SVE2 support labels May 7, 2024
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@vcsjones vcsjones removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 7, 2024
@JulieLeeMSFT JulieLeeMSFT added this to the 9.0.0 milestone May 7, 2024
@JulieLeeMSFT JulieLeeMSFT removed the untriaged New issue has not been triaged by the area owner label May 7, 2024
@kunalspathak
Copy link
Member

#104640 falls in similar category.

@a74nh
Copy link
Contributor Author

a74nh commented Jul 30, 2024

priority:3 for RC1 snap : Performance issue. Produces code that is not ideal, but will not break anything.

@a74nh a74nh added the Priority:3 Work that is nice to have label Jul 30, 2024
@a74nh a74nh removed their assignment Aug 5, 2024
@a74nh a74nh assigned SwapnilGaikwad and unassigned a74nh Aug 20, 2024
@a74nh
Copy link
Contributor Author

a74nh commented Aug 22, 2024

Given the backporting that would be required, I think it's too risky for NET9

@a74nh a74nh modified the milestones: 9.0.0, 10.0.0 Aug 22, 2024
@kunalspathak kunalspathak added Priority:2 Work that is important, but not critical for the release and removed Priority:3 Work that is nice to have labels Sep 20, 2024
SwapnilGaikwad added a commit to SwapnilGaikwad/runtime that referenced this issue Oct 25, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Oct 25, 2024
mikelle-rogers pushed a commit to mikelle-rogers/runtime that referenced this issue Dec 10, 2024
* Detect mask usage for Across functions

Fixes: dotnet#101973

* Fix build errors on x86

* Refactor containable check for csel

* Remove optimisation of AddSequentialAcross and assembly checks

* Add more tests with different mask values

* Avoid containing csel that's already containing an embedded op

* Add test for the Fuzzlyn reported failure

* Fix test build failures
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI arm-sve Work related to arm64 SVE/SVE2 support in-pr There is an active PR which will close this issue when it is merged Priority:2 Work that is important, but not critical for the release
Projects
None yet
Development

No branches or pull requests

6 participants