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

JIT: more stringent interference checks in forward sub #64933

Merged
merged 3 commits into from
Feb 8, 2022

Conversation

AndyAyersMS
Copy link
Member

Forward sub analysis was missing some cases of call arg interference and so
inviting/allowing morph to do unsafe arg reorderings.

Fix by considering all kinds of local nodes, not just GT_LCL_VAR, as sources
of interference.

Closes #64904.

Forward sub analysis was missing some cases of call arg interference and so
inviting/allowing morph to do unsafe arg reorderings.

Fix by considering all kinds of local nodes, not just GT_LCL_VAR, as sources
of interference.

Closes dotnet#64904.
@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, 2022
@ghost ghost assigned AndyAyersMS Feb 7, 2022
@ghost
Copy link

ghost commented Feb 7, 2022

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

Issue Details

Forward sub analysis was missing some cases of call arg interference and so
inviting/allowing morph to do unsafe arg reorderings.

Fix by considering all kinds of local nodes, not just GT_LCL_VAR, as sources
of interference.

Closes #64904.

Author: AndyAyersMS
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@AndyAyersMS
Copy link
Member Author

@jakobbotsch PTAL

cc @dotnet/jit-contrib

Minimal diffs on x64, more on x86, but on both, an overall improvement in code size. Presumably more evidence that when we let morph reorder call args, it has a hard time choosing a good order.

Copy link
Member

@jakobbotsch jakobbotsch left a comment

Choose a reason for hiding this comment

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

LGTM. Does this also fix the second test case in #64904?

@AndyAyersMS
Copy link
Member Author

LGTM. Does this also fix the second test case in #64904?

Yes, but that one throws an unhandled exception, and I was too lazy to convert it into a proper regression test.

@AndyAyersMS AndyAyersMS merged commit 54c910e into dotnet:main Feb 8, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Mar 11, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JIT: Incorrect results for programs in win-x86 release
3 participants