[release/9.0] JIT ARM64-SVE: Backport "Avoid combining conditional select for reduction instrinsics" (#107207)#107722
Merged
jeffschwMSFT merged 3 commits intodotnet:release/9.0from Sep 12, 2024
Conversation
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
Author
|
@kunalspathak PTAL, I had to open this manually due to merge conflicts around the ordering of |
4 tasks
jeffschwMSFT
approved these changes
Sep 12, 2024
Member
jeffschwMSFT
left a comment
There was a problem hiding this comment.
approved. we can merge when ready
Contributor
Author
|
@jeffschwMSFT could you merge this when you have a moment, please? #107725 needs to be merged on top of this PR. Thanks! |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #107207 to release/9.0
/cc @kunalspathak @SwapnilGaikwad
Customer Impact
For ARM SVE reduction intrinsics, or intrinsics that produce a scalar value, we currently do not support merging
ConditionalSelectuses into the intrinsic itself as a mask operation. So for patterns likeSve.AddAcross(Sve.ConditionalSelect(mask, vec, zero)), we only support emitting aSELto first select the correct entries in the vector, and then passing this vector toAddAcross; #101973 tracks merging these two instructions into one for .NET 10.When trying to merge
ConditionalSelectoperands of other intrinsics, the JIT did not skip this optimization for reduction intrinsics, breaking various invariants. This change ensures the JIT will not try to mergeConditionalSelectinto reduction intrinsics, and adds relevant tests.Regression
This was introduced with SVE support, which is new in .NET 9.
Testing
Added regression tests, and expanded existing SVE testing to cover more
ConditionalSelectcases.Risk
Low. This is specific to SVE support, which is experimental in .NET 9.