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/8.0-rc1] [mini] Fix typo in mono_decompose_vtype_opts #90832

Merged
merged 2 commits into from
Aug 19, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Aug 18, 2023

Backport of #90825 to release/8.0-rc1

/cc @lambdageek

Customer Impact

Customers are not able to use the new (opt-in) managed static registrar on .NET for iOS.

The registrar relies on an IL code generator to create some new managed methods that use an IL pattern that is different from what Roslyn normally generates. As a result, the IL has uncovered a bug in Mono's code generator. As a result certain equality comparisons between RuntimeTypeHandle values are compiled incorrectly and give incorrect results. The impact is that the new managed static registrar creates apps that do not work.

Testing

Manual testing.

Risk

Low/Medium. The code generator fix may have impact on other scenarios that use the Mono JIT or AOT compiler (Android, WASM). However, if this fix itself introduces a new codegen bug, it can be reverted, and .NET for iOS can fall back to the old registrar, or they can try to alter the IL pattern that their custom tool generates.

Without this, if some previous instruction already created a vreg for
ins->dest (for example if we are doing multiple passes over the basic
block because `restart == TRUE`) we will use an incorrect vreg when
decomposing the current VMOVE

Fixes #90800
This is used by a CreateSpan optimization that needs access to the
MonoClassField*

For other cases of a bare LDTOKEN (such as hand-written IL that calls
LDTOKEN on a type but doesn't follow it up with a call to
`GetTypeFromHandle` leave the opcode as a VMOVE (from the
`EMIT_NEW_TEMPLOAD` above))
@carlossanlop
Copy link
Member

We already started the builds for RC1. Please fill out the template, send an email to Tactics requesting approval, get a code review sign off, and please let me know when the CI finishes so I can ask if we can still merge.

@lambdageek lambdageek added this to the 8.0.0 milestone Aug 18, 2023
@lambdageek lambdageek added the Servicing-consider Issue for next servicing release review label Aug 18, 2023
@lewing lewing added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Aug 19, 2023
@carlossanlop carlossanlop merged commit 6aef01d into release/8.0-rc1 Aug 19, 2023
103 of 109 checks passed
@carlossanlop carlossanlop deleted the backport/pr-90825-to-release/8.0-rc1 branch August 19, 2023 02:33
@ghost ghost locked as resolved and limited conversation to collaborators Sep 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Codegen-JIT-mono Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants