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-staging][mono][interp] Fix inlining of ldarga #97650

Conversation

BrzVlad
Copy link
Member

@BrzVlad BrzVlad commented Jan 29, 2024

When inlining a method, all arguments are first copied to new vars. Ldarga is going to be applied on these copies. We were loading the address of the wrong var due to typo.

Backport of #97553

Customer Impact

Inlining of methods containing the ldarga opcode is broken on mono interpreter. This was reported as a regression from .net 7 (on Blazor WASM project), indirectly caused by an increase of the inline limit for methods.

Testing

Tested on customer provided sample.

Risk

Very low. The fix is isolated to the code gen for ldarga opcode during inlining, which was completely broken before.

When inlining a method, all arguments are first copied to new vars. Ldarga is going to be applied on these copies. We were loading the address of the wrong var due to typo.
@ghost
Copy link

ghost commented Jan 29, 2024

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

Issue Details

When inlining a method, all arguments are first copied to new vars. Ldarga is going to be applied on these copies. We were loading the address of the wrong var due to typo.

Backport of #97553

Customer Impact

Inlining of methods containing the ldarga opcode is broken on mono interpreter. This was reported as a regression from .net 7 (on Blazor WASM project), indirectly caused by an increase of the inline limit for methods.

Testing

Tested on customer provided sample.

Risk

Very low. The fix is isolated to the code gen for ldarga opcode during inlining, which was completely broken before.

Author: BrzVlad
Assignees: -
Labels:

area-Codegen-Interpreter-mono

Milestone: -

@BrzVlad
Copy link
Member Author

BrzVlad commented Jan 29, 2024

@lewing

@lewing lewing added the Servicing-consider Issue for next servicing release review label Jan 29, 2024
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 30, 2024
@rbhanda rbhanda added this to the 8.0.3 milestone Jan 30, 2024
@jeffschwMSFT jeffschwMSFT merged commit 2767a15 into dotnet:release/8.0-staging Feb 9, 2024
108 of 111 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants