-
Notifications
You must be signed in to change notification settings - Fork 5.1k
JIT: Fix ARM64 emitter tests #117768
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
base: main
Are you sure you want to change the base?
JIT: Fix ARM64 emitter tests #117768
Conversation
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.
Pull Request Overview
This PR disables unit tests for unsupported SVE encodings and corrects incorrect branch conditions in the ARM64 SVE emitter.
- Wrap unsupported encoding tests in
codegenarm64test.cpp
withALL_ARM64_EMITTER_UNIT_TESTS_SVE_UNSUPPORTED
guards. - Fix conditional checks in
emitarm64sve.cpp
to useINS_OPTS_SCALABLE_S
andINS_OPTS_SCALABLE_D
in the correct branches.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/coreclr/jit/emitarm64sve.cpp | Corrected conditional checks for INS_OPTS_SCALABLE_S /_D in SVE emitter format logic |
src/coreclr/jit/codegenarm64test.cpp | Wrapped unsupported SVE encoding tests in #ifdef ALL_ARM64_EMITTER_UNIT_TESTS_SVE_UNSUPPORTED |
@@ -4685,11 +4685,13 @@ void CodeGen::genArm64EmitterUnitTestsSve() | |||
theEmitter->emitIns_R_R_R(INS_sve_urshlr, EA_SCALABLE, REG_V15, REG_P2, REG_V20, | |||
INS_OPTS_SCALABLE_D); // URSHLR <Zdn>.<T>, <Pg>/M, <Zdn>.<T>, <Zm>.<T> | |||
|
|||
#ifdef ALL_ARM64_EMITTER_UNIT_TESTS_SVE_UNSUPPORTED |
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.
[nitpick] Consider grouping the multiple unsupported encoding test blocks under a single #ifdef ALL_ARM64_EMITTER_UNIT_TESTS_SVE_UNSUPPORTED
region to improve readability and reduce repetitive macro guards.
Copilot uses AI. Check for mistakes.
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
@dotnet/jit-contrib PTAL, thanks |
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.
This LGTM. Revert of a recent change plus some fixups.
Follow-up to discussion in #117660. This disables tests for unsupported encodings, and fixes a few incorrect branches in the SVE emitter. cc @dotnet/arm64-contrib