-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
JIT ARM64-SVE: Add BitwiseClear and BooleanNot #101853
Conversation
Note regarding the
|
Tagging subscribers to this area: @dotnet/area-system-runtime-intrinsics |
@@ -3602,12 +3602,7 @@ void emitter::emitInsSve_R_R_R(instruction ins, | |||
fmt = IF_SVE_GW_3A; | |||
break; | |||
|
|||
case INS_sve_clz: |
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.
The asserts for these instructions were wrong - it allowed cases that don't exist.
@dotnet/arm64-contrib @kunalspathak |
/azp run runtime-coreclr superpmi-replay, runtime-coreclr superpmi-diffs |
Azure Pipelines successfully started running 2 pipeline(s). |
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.
nit: Suggested change to remove instruction from summary docs.
/// BIC Ztied1.B, Pg/M, Ztied1.B, Zop2.B | ||
/// BIC Zresult.D, Zop1.D, Zop2.D | ||
/// svbool_t svbic[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2) | ||
/// BIC Presult.B, Pg/Z, Pop1.B, Pop2.B |
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.
I don't think we are supporting the BIC Presult.B, Pg/Z, Pop1.B, Pop2.B
. Can we remove it from the summary docs in this and other APIs?
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.
Sve. BitwiseClear(mask)
will convert the mask to a normal vector and use the vector BIC instruction. That's not great. We should be using the mask variant where possible. We need something in lowering? hwinstrinsics? to spot this.
I've raised an issue to cover it: #101970
Is it ok to leave the summary as is, because we'll have to restore them when fixing the issue. Plus it makes it easier to spot them when we fix it.
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.
sure
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
Test results: