-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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 conditional select for Zeroing predicates #102904 #105737
Conversation
Stress test results:
|
@dotnet/arm64-contrib @kunalspathak |
ping @tannergooding |
Done! Thanks. |
ping @tannergooding |
// If the nested op uses Pg/Z, then inactive lanes will result in zeros, so can only transform if | ||
// op3 is all zeros. | ||
|
||
if (nestedOp1->IsMaskAllBitsSet() && | ||
(!HWIntrinsicInfo::IsZeroingMaskedOperation(nestedOp2Id) || op3->IsVectorZero())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any instructions that only allow Pg/M
?
Basically we have:
- Pg/Z only
- Pg/M -or- Pg/Z
So I'm wanting to discern if we also have:
- Pg/M only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, quite a few.
eg:
https://docsmirror.github.io/A64/2023-09/add_z_p_zz.html
ADD <Zdn>.<T>, <Pg>/M, <Zdn>.<T>, <Zm>.<T>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We likely need something that indicates that and avoids containment of zero for that case then, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's <Pg>/M
only then the optimisation will continue to work the same as it does in HEAD (because IsZeroingMaskedOperation()
will be false)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM assuming we don't have any Pg/M
only instructions
Fixes #102904
Some SVE instructions use Pg/Z instead of Pg/M.
This means they cannot be wrapped in conditional select in the standard way - if they are then any inactive lanes will end up with zeros instead of the false value.
For example:
Sve.ConditionalSelect(mask, Sve.LoadVector(address), falseOp);
Cannot be converted into a single load:
LD1W zFalseVal, pMask/Z, xAddress
Instead two instructions are required:
Fix by adding marking all relevant instructions and disabling the condition select optimisation for these instructions.