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

Constrained mask should take into account busy registers #85933

Merged
merged 12 commits into from
May 16, 2023

Conversation

kunalspathak
Copy link
Member

@kunalspathak kunalspathak commented May 8, 2023

When we constrained the registers under JitStressRegs, check if the constrained masks is not left with "busy registers" because in that case, we will not be able to allocate register. Relax that and do not constraint if they coincides with "busy registers".

Fixes: #85627
(Most likely) fixes: #76382

Also noticed a problem where we were not taking into account SUB_HI and LSH_HI for RMW analysis. Doing that discovered that we should update the assert about Commutative() to take into account if both operands are same. Both of these will fix the following issue:
Fixes: #59916

@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 May 8, 2023
@ghost ghost assigned kunalspathak May 8, 2023
@ghost
Copy link

ghost commented May 8, 2023

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

Issue Details

When we constrained the registers under JitStressRegs, check if the constrained masks is not left with "busy registers" because in that case, we will not be able to allocate register. Relax that and do not constraint if they coincides with "busy registers".

Fixes: #85627
(Most likely) fixes: #76382

Author: kunalspathak
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@kunalspathak
Copy link
Member Author

I am seeing superpmi-replay failures similar to one reported in #59916.

@kunalspathak kunalspathak marked this pull request as ready for review May 13, 2023 02:34
@kunalspathak
Copy link
Member Author

@dotnet/jit-contrib

Copy link
Member

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

Mostly nits but one question

lclNum2 = op2Skip->AsLclVarCommon()->GetLclNum();
}
#endif
noway_assert(GenTree::OperIsCommutative(oper) || (lclNum1 == lclNum2));
Copy link
Member

Choose a reason for hiding this comment

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

I realized this can't be a noway_assert now: it would mean we're asserting different things in Release and Debug: and the case where you're asserting lclNum1 == lclNum2 would fail in Release.

@ghost ghost added the needs-author-action An issue or pull request that requires more info or actions from the author. label May 16, 2023
@ghost ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label May 16, 2023
@kunalspathak kunalspathak merged commit 17bf9b8 into dotnet:main May 16, 2023
@kunalspathak kunalspathak deleted the constrained-mask branch May 16, 2023 21:04
@ghost ghost locked as resolved and limited conversation to collaborators Jun 16, 2023
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
Projects
None yet
2 participants