Skip to content
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

Add ARM64 encodings for group IF_SVE_CT_3A #98085

Merged
merged 3 commits into from
Feb 8, 2024

Conversation

snickolls-arm
Copy link
Contributor

Adds support for encoding revd. Matching capstone:

revd z1.q, p0/m, z6.q
revd z5.q, p7/m, z12.q
revd z7.q, p3/m, z11.q
revd z28.q, p4/m, z3.q

Contributing towards #94549.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 7, 2024
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Feb 7, 2024
@ghost
Copy link

ghost commented Feb 7, 2024

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

Adds support for encoding revd. Matching capstone:

revd z1.q, p0/m, z6.q
revd z5.q, p7/m, z12.q
revd z7.q, p3/m, z11.q
revd z28.q, p4/m, z3.q

Contributing towards #94549.

Author: snickolls-arm
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@snickolls-arm
Copy link
Contributor Author

@a74nh @kunalspathak @dotnet/arm64-contrib

@snickolls-arm snickolls-arm marked this pull request as ready for review February 7, 2024 13:13
@kunalspathak kunalspathak added the arm-sve Work related to arm64 SVE/SVE2 support label Feb 7, 2024
@@ -1102,6 +1102,12 @@ void emitter::emitInsSanityCheck(instrDesc* id)
assert(isPredicateRegister(id->idReg3())); // MMMM
break;

case IF_SVE_CT_3A: // ................ ...gggnnnnnddddd -- SVE reverse doublewords
assert(isVectorRegister(id->idReg3())); // ddddd
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have these in order of reg1, reg2 and reg3

@@ -4754,6 +4754,12 @@ void CodeGen::genArm64EmitterUnitTestsSve()
theEmitter->emitIns_R_R_R(INS_sve_clastb, EA_8BYTE, REG_R3, REG_P6, REG_V9,
INS_OPTS_SCALABLE_D); // CLASTB <R><dn>, <Pg>, <R><dn>, <Zm>.<T>

// IF_SVE_CT_3A
theEmitter->emitIns_R_R_R(INS_sve_revd, EA_SCALABLE, REG_V1, REG_P0, REG_V6); // REVD <Zd>.Q, <Pg>/M, <Zn>.Q
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since all the tests are identical, 1 or 2 cases should be sufficient enough.

@@ -1102,6 +1102,12 @@ void emitter::emitInsSanityCheck(instrDesc* id)
assert(isPredicateRegister(id->idReg3())); // MMMM
break;

case IF_SVE_CT_3A: // ................ ...gggnnnnnddddd -- SVE reverse doublewords
assert(isVectorRegister(id->idReg3())); // ddddd
assert(isPredicateRegister(id->idReg2())); // ggg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert(isPredicateRegister(id->idReg2())); // ggg
assert(isLowPredicateRegister(id->idReg2())); // ggg

@@ -10017,6 +10023,13 @@ void emitter::emitIns_R_R_R(instruction ins,
}
break;

case INS_sve_revd:
assert(isVectorRegister(reg1)); // ddddd
assert(isPredicateRegister(reg2)); // ggg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert(isPredicateRegister(reg2)); // ggg
assert(isLowPredicateRegister(reg2)); // ggg

@ghost ghost added the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 7, 2024
@ghost ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 7, 2024
@ryujit-bot
Copy link

Diff results for #98085

Throughput diffs

Throughput diffs for linux/arm64 ran on linux/x64

MinOpts (-0.00% to +0.01%)
Collection PDIFF
libraries.pmi.linux.arm64.checked.mch +0.01%

Throughput diffs for windows/arm64 ran on linux/x64

MinOpts (-0.00% to +0.01%)
Collection PDIFF
libraries.pmi.windows.arm64.checked.mch +0.01%

Details here


Copy link
Member

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@ryujit-bot
Copy link

Diff results for #98085

Throughput diffs

Throughput diffs for linux/arm64 ran on windows/x64

MinOpts (-0.00% to +0.01%)
Collection PDIFF
libraries.pmi.linux.arm64.checked.mch +0.01%

Throughput diffs for windows/arm64 ran on windows/x64

MinOpts (-0.00% to +0.01%)
Collection PDIFF
libraries.pmi.windows.arm64.checked.mch +0.01%

Details here


Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI arm-sve Work related to arm64 SVE/SVE2 support community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants