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

arm64: Add tests for add(s), and(s), sub(s), cmp, cmn, eor, neg & orr #111796

Conversation

jonathandavies-arm
Copy link
Contributor

No description provided.

@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 Jan 24, 2025
@jonathandavies-arm
Copy link
Contributor Author

@a74nh @kunalspathak @dotnet/arm64-contrib

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jan 24, 2025
Copy link
Contributor

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

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.

Thanks for adding the test coverage. Added some follow up questions.

static bool CmnLSR(uint a, uint b)
{
//ARM64-FULL-LINE: lsr {{w[0-9]+}}, {{w[0-9]+}}, #2
//ARM64-FULL-LINE: cmn {{w[0-9]+}}, {{w[0-9]+}}
Copy link
Member

Choose a reason for hiding this comment

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

can we open an issue for this? According to #68028, "shifted register" should already be in place.

Copy link
Member

Choose a reason for hiding this comment

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

Do we know why it doesn't kick in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

{
//ARM64-FULL-LINE: lsl {{x[0-9]+}}, {{x[0-9]+}}, #55
//ARM64-FULL-LINE: cmp {{x[0-9]+}}, {{x[0-9]+}}
if (a == (b<<119))
Copy link
Member

Choose a reason for hiding this comment

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

are we having two instructions here because the constant doesn't fit in imm6 of cmp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

static int EorLSL(int a, int b)
{
//ARM64-FULL-LINE: eor {{w[0-9]+}}, {{w[0-9]+}}, {{w[0-9]+}}, LSL #4
return a ^ (b<<4);
Copy link
Member

Choose a reason for hiding this comment

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

can you add some test for (b << 4) ^ a?

Copy link
Member

Choose a reason for hiding this comment

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

similar for other commutative operations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added tests with operands in opposite order.

{
//ARM64-FULL-LINE: lsr {{w[0-9]+}}, {{w[0-9]+}}, #20
//ARM64-FULL-LINE: neg {{w[0-9]+}}, {{w[0-9]+}}
return (uint)-(a>>20);
Copy link
Member

Choose a reason for hiding this comment

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

do we know why we generate 2 instructios here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

@kunalspathak kunalspathak merged commit bcc0e89 into dotnet:main Jan 29, 2025
73 checks passed
@jonathandavies-arm jonathandavies-arm deleted the upsteam/compact-encodings-more-tests branch January 30, 2025 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-arm64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI 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.

2 participants