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

Another size estimate fix for movs #64826

Merged

Conversation

SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Feb 4, 2022

This time, we were overestimating mov [addr], imm (by 4 bytes).

Another small step towards CLS_VAR deletion.

A small number of positive diffs.

@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 4, 2022
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Feb 4, 2022
@ghost
Copy link

ghost commented Feb 4, 2022

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

Issue Details

This, time, we were overestimating mov [addr], imm (by 4 bytes).

Another small step towards CLS_VAR deletion.

We are expecting a small number of positive diffs.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr, community-contribution

Milestone: -

Comment on lines -2783 to +2789
// 64-bit immediates are only supported on mov r64, imm64
// As per manual:
// Support for 64-bit immediate operands is accomplished by expanding
// the semantics of the existing move (MOV reg, imm16/32) instructions.
if ((valSize > sizeof(INT32)) && (ins != INS_mov))
valSize = sizeof(INT32);
#else
// occasionally longs get here on x86
// mov reg, imm64 is the only opcode which takes a full 8 byte immediate
// all other opcodes take a sign-extended 4-byte immediate
noway_assert(valSize <= sizeof(INT32) || !id->idIsCnsReloc());
Copy link
Contributor Author

@SingleAccretion SingleAccretion Feb 4, 2022

Choose a reason for hiding this comment

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

mov reg, imm64 does not get here as this is for the M-format instructions, so there is no need to check for it.

Also, this comment is intentionally copied verbatim from emitInsSizeAM.

@SingleAccretion SingleAccretion marked this pull request as ready for review February 6, 2022 15:36
@SingleAccretion
Copy link
Contributor Author

@dotnet/jit-contrib

@BruceForstall BruceForstall merged commit 829fe59 into dotnet:main Feb 8, 2022
@SingleAccretion SingleAccretion deleted the Fix-Size-Estimate-For-Movs branch February 9, 2022 19:08
@ghost ghost locked as resolved and limited conversation to collaborators Mar 12, 2022
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 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