Open
Conversation
First part of dotnet#94425 and dotnet#94426 When adding a new group, I've added the entire group for completeness. Plus fix up existing code: * umull/smull etc does not have a 32bit version * stlurb etc uses scalar registers, not vector registers * Fix the missing bit for simd ldapur * smaddl etc does not have a 32bit version * sha1su1/sha256su0 were in the wrong groups, and had no tests * cdot test immediate values were wrong * fix some register display issues
github-actions
bot
added
the
area-CodeGen-coreclr
dotnet-policy-service
bot
added
the
community-contribution
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support in the CoreCLR ARM64 JIT emitter for additional crypto instructions (SM4 and SHA3/SHA512/SM3-related), along with several correctness fixes in existing instruction metadata/encoding and expanded emitter unit tests to validate the new/adjusted encodings.
Changes:
- Add new ARM64 instruction definitions and emitter formats for SHA3/SHA512/SM3/SM4-related instructions.
- Fix several existing instruction/emitter issues (e.g., incorrect operand sizing for *mull/*maddl families, scalar vs vector register usage for some loads/stores, and a missing vector LDAPUR encoding bit).
- Extend ARM64 emitter unit tests (including corrections to SVE
cdotimmediate values) to cover newly added/relocated instruction handling.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/coreclr/jit/instrsarm64.h |
Adds instruction table entries for new crypto ops; adjusts SHA instruction grouping/format usage. |
src/coreclr/jit/emitfmtsarm64.h |
Introduces new emitter formats (DV_2V, DV_3H, DV_3I, DV_4B) to support new instruction encodings. |
src/coreclr/jit/emitarm64.cpp |
Wires new formats into sanity checks, encoding output, disassembly display, and perf characteristics; fixes various existing emitter behaviors. |
src/coreclr/jit/codegenarmarch.cpp |
Fixes ARM64 GT_MUL_LONG emission to use correct 64-bit result size for long-mul instructions. |
src/coreclr/jit/codegenarm64.cpp |
Fixes ARM64 MulHi codegen to use correct operand/result sizing for smull/umull. |
src/coreclr/jit/codegenarm64test.cpp |
Updates/extends emitter unit tests for corrected encodings and newly added instructions; fixes SVE cdot immediates. |
Contributor
Author
|
@dotnet/arm64-contrib |
Contributor
Author
|
There are some disasm test failures - I suspect it's just differences in the way the assembly is printed. I'll take a look. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Contributor
Author
|
Looks like that's it for all the copilot fixups (mostly comment changes). I'm happy with this PR now. |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
First part of #94425 and #94426
When adding a new group, I've added the entire group for completeness.
Plus fix up existing code: