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: No obvious way to mask an Across function #101770

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

ARM64-SVE: No obvious way to mask an Across function #101770

a74nh opened this issue May 1, 2024 · 7 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
Milestone

Comments

@a74nh
Copy link
Contributor

a74nh commented May 1, 2024

ConditionalSelect is used to mask an API, for example:

a = Sve.ConditionalSelect(mask, Sve.Add(a,b), a);

Is equivalent to:

ADD zA, zMask/M, zA, zB

This does not work when using the across methods.

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

Is saying to add together all the elements in a and place in field 0, zeroing all other elements. Then use mask to select which elements to pick from the result. That is valid C#, but is a weird thing to do.

the more likely use case is to take all the elements in a that are marked as active in mask and add them together, putting them in field 0, zeroing all other elements. There is no obvious way to do this?

@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 1, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label May 1, 2024
@a74nh
Copy link
Contributor Author

a74nh commented May 1, 2024

Having said that, it now occurs to me that for AddAcross you could do:

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

But that doesn't work for all Across methods. For example, MinAcross().

@a74nh
Copy link
Contributor Author

a74nh commented May 1, 2024

@kunalspathak @tannergooding - this came up as part of implementing #101674

@kunalspathak
Copy link
Member

We discussed it in #101674 (comment)

@tannergooding
Copy link
Member

tannergooding commented May 1, 2024

Cases like MinAcross could be handled by doing Sve.MinAcross(Sve.ConditionalSelect(mask, a, Vector.Create(max)) (where max for float is +Infinity). That being said, predication of "across" APIs is in general a more complex case and so may be one of the scenarios that warrants having an overload that explicitly takes the mask.

We simply want to prefer ConditionSelect in the cases where pattern matching and the pattern a user would need to write is "obvious" as it saves us from needing to expose thousands of additional API overloads. It also has the added benefit that it matches the patterns users already have to write for ISAs without built-in masking support, such as AdvSimd (NEON), and so allows existing code to be shared across these ISAs and to implicitly light up, making the entire ecosystem trivially better.

@a74nh
Copy link
Contributor Author

a74nh commented May 1, 2024

I think in these cases extra API methods would make sense.

This probably needs splitting into two issues then:

  1. An API issue with the new methods in it
  2. For the cases where the user does write code in the *Across(ConditionalSelect) style, something in lowering to detect and optimise.

@jeffschwMSFT jeffschwMSFT added arm-sve Work related to arm64 SVE/SVE2 support area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels May 2, 2024
Copy link
Contributor

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

@teo-tsirpanis teo-tsirpanis removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 3, 2024
@JulieLeeMSFT JulieLeeMSFT added this to the 9.0.0 milestone May 3, 2024
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label May 3, 2024
@a74nh
Copy link
Contributor Author

a74nh commented May 7, 2024

Opened:

Closing this issue in favour of those

@a74nh a74nh closed this as completed May 7, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jun 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
Projects
None yet
Development

No branches or pull requests

6 participants