Add ARM64 encodings for group IF_SVE_GQ_3A #98352
Merged
Conversation
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
ghost
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsAdds encodings for bfcvtnt, fcvtlt, fcvtnt and fcvtxnt. Contributing towards #94549.
|
kunalspathak
changed the title
kunalspathak
suggested changes
Feb 13, 2024
src/coreclr/jit/emitarm64.h
Outdated
|
|
||
| // Expands an option that has different size operands (INS_OPTS_*_TO_*) into a pair of scalable options where | ||
| // the first describes the size of the destination operand and the second describes the size of the source operand. | ||
| std::pair<insOpts, insOpts> optExpandConversionPair(insOpts opt); |
There was a problem hiding this comment.
we do not use standard libraries function like std::pair. Can you instead change it to something like
Suggested change
| std::pair<insOpts, insOpts> optExpandConversionPair(insOpts opt); | |
| void optExpandConversionPair(insOpts opt, insOpts& dst, insOpts& src); |
src/coreclr/jit/emitarm64.cpp
Outdated
| case INS_sve_fcvtnt: | ||
| if (id->idInsOpt() == INS_OPTS_S_TO_H) | ||
| { | ||
| code ^= (1 << 22 | 1 << 17); |
There was a problem hiding this comment.
can you instead change the below to:
runtime/src/coreclr/jit/instrsarm64sve.h
Line 1316 in eb0c20c
- INST2(fcvtnt, "fcvtnt", 0, IF_SVE_2BJ, 0x64CAA000, 0x650A3C00 )
- // FCVTNT <Zd>.S, <Pg>/M, <Zn>.D SVE_GQ_3A 0110010011001010 101gggnnnnnddddd 64CA A000
+ INST2(fcvtnt, "fcvtnt", 0, IF_SVE_2BJ, 0x6488A000, 0x650A3C00 )
+ // FCVTNT <Zd>.H, <Pg>/M, <Zn>.S SVE_GQ_3A 0110010010001000 101gggnnnnnddddd 6488 A000 and then use |:
Suggested change
| code ^= (1 << 22 | 1 << 17); | |
| code |= (1 << 22 | 1 << 17); |
src/coreclr/jit/emitarm64.cpp
Outdated
| case INS_sve_fcvtlt: | ||
| if (id->idInsOpt() == INS_OPTS_H_TO_S) | ||
| { | ||
| code ^= (1 << 22 | 1 << 17); |
src/coreclr/jit/emitarm64.cpp
Outdated
| case INS_sve_fcvtxnt: | ||
| case INS_sve_bfcvtnt: | ||
| assert(isVectorRegister(reg1)); // ddddd | ||
| assert(isPredicateRegister(reg2)); // ggg |
There was a problem hiding this comment.
Suggested change
| assert(isPredicateRegister(reg2)); // ggg | |
| assert(isLowPredicateRegister(reg2)); // ggg |
There was a problem hiding this comment.
and at other places too, the assert should be for LowPredicateRegister().
|
@a74nh @kunalspathak @dotnet/arm64-contrib |
|
Failures are known and superpmi-x64 is clean. |
Diff results for #98352Throughput diffsThroughput diffs for linux/arm64 ran on windows/x64MinOpts (-0.01% to +0.00%)
Throughput diffs for osx/arm64 ran on windows/x64MinOpts (-0.01% to +0.00%)
Throughput diffs for windows/arm64 ran on windows/x64MinOpts (-0.00% to +0.01%)
Details here |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
Adds encodings for bfcvtnt, fcvtlt, fcvtnt and fcvtxnt.
Contributing towards #94549.