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: Reimport full spill clique for I_IMPL<->BYREF mismatches #92307

Merged

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Sep 19, 2023

When the JIT sees code that pushes an untracked pointer from one branch,
and a tracked pointer from another, it will reimport successors that
were already imported under the wrong assumption of an untracked
pointer. However, like in the other cases of spill clique handling, it
should reimport the entire spill clique, including predecessors and other
successors of those; otherwise we can still end up with mistyped LCL_VAR
nodes.

Block morphing would create oddly typed trees (mixing up
TYP_BYREF/TYP_I_IMPL, e.g. creating LCL_VAR<I_IMPL> for a TYP_BYREF
typed local). The only effect of this was that it would avoid some
constant propagation. Make this more explicit by setting GTF_DONT_CSE
instead.
When the JIT sees code that pushes an untracked pointer from one branch,
and a tracked pointer from another, it will reimport successors that
were already imported under the wrong assumption of an untracked
pointer. However, like in the other cases of spill clique handling, it
should reimport the entire spill clique, including predecssors and other
successors of those; otherwise we can still end up with mistyped LCL_VAR
nodes.
@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 Sep 19, 2023
@ghost ghost assigned jakobbotsch Sep 19, 2023
@ghost
Copy link

ghost commented Sep 19, 2023

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

Issue Details

When the JIT sees code that pushes an untracked pointer from one branch,
and a tracked pointer from another, it will reimport successors that
were already imported under the wrong assumption of an untracked
pointer. However, like in the other cases of spill clique handling, it
should reimport the entire spill clique, including predecssors and other
successors of those; otherwise we can still end up with mistyped LCL_VAR
nodes.

Based on #92292 since that PR is also necessary to allow us to restrict the assert in lvaMarkLclRefs.

Author: jakobbotsch
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch
Copy link
Member Author

/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@jakobbotsch jakobbotsch marked this pull request as ready for review September 22, 2023 18:57
@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib PTAL @AndyAyersMS

No diffs. Failure is known according to build analysis.

@jakobbotsch jakobbotsch merged commit 54bdc3f into dotnet:main Sep 24, 2023
164 of 166 checks passed
@jakobbotsch jakobbotsch deleted the lcl-type-mismatch-reimport-spill-clique branch September 24, 2023 21:31
@ghost ghost locked as resolved and limited conversation to collaborators Oct 25, 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
Development

Successfully merging this pull request may close these issues.

3 participants