ARM64-SVE: Fix hwintrinsics flags #107791
Merged
Conversation
This file contains 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
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
dotnet-policy-service
bot
added
the
community-contribution
a74nh
added
arm-sve
|
This is ready. @dotnet/arm64-contrib @kunalspathak |
|
Difference between this PR and HEAD is that HEAD will mark some registers as delay free when it doesn't need to. |
|
Confirmed stress tests of the hwintrinsics tests fully pass. |
kunalspathak
approved these changes
Sep 13, 2024
| @@ -204,7 +204,8 @@ enum HWIntrinsicFlag : unsigned int | |||
| // The intrinsic uses a mask in arg1 to select elements present in the result | |||
| HW_Flag_ExplicitMaskedOperation = 0x20000, | |||
|
|
|||
| // The intrinsic uses a mask in arg1 to select elements present in the result, and must use a low register. | |||
| // The intrinsic uses a mask in arg1 (either explicitly, embdedd or optionally embedded) to select elements present | |||
There was a problem hiding this comment.
nit: something to fix in follow-up PR
Suggested change
| // The intrinsic uses a mask in arg1 (either explicitly, embdedd or optionally embedded) to select elements present | |
| // The intrinsic uses a mask in arg1 (either explicitly, embedded or optionally embedded) to select elements present |
| HARDWARE_INTRINSIC(Sve, ConvertMaskToVector, -1, 1, {INS_sve_mov, INS_sve_mov, INS_sve_mov, INS_sve_mov, INS_sve_mov, INS_sve_mov, INS_sve_mov, INS_sve_mov, INS_sve_mov, INS_sve_mov}, HW_Category_Helper, HW_Flag_Scalable|HW_Flag_ExplicitMaskedOperation) | ||
| HARDWARE_INTRINSIC(Sve, ConvertVectorToMask, -1, 2, {INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne}, HW_Category_Helper, HW_Flag_Scalable|HW_Flag_ReturnsPerElementMask|HW_Flag_LowMaskedOperation) | ||
| HARDWARE_INTRINSIC(Sve, ConvertMaskToVector, -1, 1, {INS_sve_mov, INS_sve_mov, INS_sve_mov, INS_sve_mov, INS_sve_mov, INS_sve_mov, INS_sve_mov, INS_sve_mov, INS_sve_mov, INS_sve_mov}, HW_Category_Helper, HW_Flag_Scalable) | ||
| HARDWARE_INTRINSIC(Sve, ConvertVectorToMask, -1, 2, {INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne, INS_sve_cmpne}, HW_Category_Helper, HW_Flag_Scalable|HW_Flag_ExplicitMaskedOperation|HW_Flag_ReturnsPerElementMask|HW_Flag_LowMaskedOperation) |
There was a problem hiding this comment.
removing HW_Flag_ExplicitMaskedOperation from MaskToVector and putting it VectorToMask makes sense, but wondering why it worked so far? probably we do not care of this flag and can be eliminated?
There was a problem hiding this comment.
spoke offline. As expected, this was never used for these 2 intrinsics, but good to have for correctness.
There was a problem hiding this comment.
We'll never use it because the converts will never be embedded
|
/backport to release/9.0 |
|
Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/10853493972 |
4 tasks
jtschuster
pushed a commit
to jtschuster/runtime
that referenced
this pull request
Sep 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
ARM64-SVE: Fix hwintrinsics flags
Spotted during implementation of #107459
SQADD and SQSUB have two variants, predicated and unpredicated. Currently we only support SVE1, so must use the unpredicated version. Remove RMW and low mask flags, because these are for the predicated version.
ConvertMaskToVector does not have an explicit mask, but ConvertVectorToMask does. Fix these.
Add assert checks to IsLowMaskedOperation