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

[release/6.0] Fix gtCloneExpr when cloning during R2R compilation a GT_ALLOCOBJ node #59421

Closed
wants to merge 3 commits into from

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 21, 2021

Backport of #59395 to release/6.0

/cc @davidwrighton

Customer Impact

  • Without this fix cloned expressions with allocations will fail
  • This is most common in profile guided code around devirtualization, but I believe it can occur in other where gtCloneExpr is used
  • Symptom of the failure is a compilation failure during crossgen2
  • This blocks usage of static PGO technology on larger applications which have code patterns like
localVariable.CallSomeVirtualMethod(new SomeType());

Under conditions I am unable to identify that will sometimes cause the JIT to generate data structures vulnerable to this bug if the type of localVariable can be predicted via PGO.

Testing

Standard PR run + targeted testing of the PGO scenario which failed without this fix.

Risk

Low. This fixes a logic error in the jit that would cause prejitting of a method to fail.

…e - Without this fix cloned expressions with allocations will fail - This is most common in profile guided code around devirtualization, but I believe it can occur in other where gtCloneExpr is used - Symptom of the failure is a compilation failure during crossgen2
@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 21, 2021
@ghost
Copy link

ghost commented Sep 21, 2021

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

Issue Details

Backport of #59395 to release/6.0

/cc @davidwrighton

Customer Impact

Testing

Risk

Author: github-actions[bot]
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@davidwrighton davidwrighton added the Servicing-consider Issue for next servicing release review label Sep 21, 2021
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

Approved. Can you update the customer impact to highlight a pattern that runs into this and elaborate a bit more on prevalence?

@jeffschwMSFT
Copy link
Member

@davidwrighton we are still considering fixes for RC2. I think we should consider it for RC2, do you?

@davidwrighton
Copy link
Member

@jeffschwMSFT I thought we had missed that cutoff. I'll put together the RC2 PR as well.

@jeffschwMSFT
Copy link
Member

We are really close. You might want to request approval for this including considering for RC2.

@davidwrighton
Copy link
Member

/backport to release/6.0-rc2

@github-actions
Copy link
Contributor Author

Started backporting to release/6.0-rc2: https://github.com/dotnet/runtime/actions/runs/1259619216

@davidwrighton
Copy link
Member

This was merged into RC2 directly via #59438 instead. Thus I'm closing this PR.

@davidwrighton davidwrighton deleted the backport/pr-59395-to-release/6.0 branch September 22, 2021 18:45
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
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 Servicing-consider Issue for next servicing release review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants