- Notifications
You must be signed in to change notification settings - Fork5.2k
[release/9.0-staging] [mono][interp] Minor SSA fixes#116428
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…during cprop (dotnet#116179)The definition was not updated, leading to invalid optimizations later on.
… optimization (dotnet#116069)For each instruction in a basic block we check for patterns. In a certain case, once we replaced the instruction with a new one, we were attempting to do a loop reiteration by setting `ins = ins->prev` so after the loop reincrements with `ins = ins->next` we check super instruction patterns again for the current instruction. This is broken if `ins` was the first instruction in a basic block, aka `ins->prev` is NULL. This used to be impossible before SSA optimizations, since super instruction pass was applying optimizations in a single basic block only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
Backports two localized SSA fixes for the interpreter and adds configurable opt-out via an environment variable.
- Ensures SSA def-tracking is updated after clearing old instructions in constant propagation.
- Refines super-instruction logic to re-evaluate opcodes after a merge step.
- Introduces
MONO_INTERPRETER_OPTIONSenv var to override interpreter options at runtime.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/mono/mono/mini/interp/transform-opt.c | Update SSA def tracking and adjust super-instruction retry logic |
| src/mono/mono/mini/interp/interp.c | Parse and applyMONO_INTERPRETER_OPTIONS from environment |
Comments suppressed due to low confidence (1)
src/mono/mono/mini/interp/interp.c:8985
- [nitpick] No tests currently cover parsing and application of
MONO_INTERPRETER_OPTIONS. Adding a test case that sets this env var and verifies options parsing would guard against regressions.
const char *env_opts = g_getenv ("MONO_INTERPRETER_OPTIONS");| if (td->verbose_level) { | ||
| g_print ("superins: "); | ||
| interp_dump_ins (ins,td->data_items); | ||
| } |
CopilotAIJun 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The newgoto retry_ins no longer adjustsins (original code didins = ins->prev) so the intended behavior of re-evaluating theprevious instruction is lost. Consider restoringins = ins->prev before retry or clarifying the control flow to avoid changing logic or introducing infinite loops.
| } | |
| } | |
| ins=ins->prev;// Adjust to re-evaluate the previous instruction |
jozkee commentedJun 9, 2025
@BrzVlad reminder that servicing code complete for 9.0.7 is today @ 3PM PST. |
jozkee commentedJun 10, 2025
/ba-g all new test failures are WasmTestOn* timeouts,#116444, the one now showing due to space limitations I believe isWorkloads-NoWebcil-ST-Wasm.Build.Tests.WasmTemplateTests, also timeout. |
2ec664a intodotnet:release/9.0-stagingUh oh!
There was an error while loading.Please reload this page.
Emanuele9xx commentedJun 13, 2025
When will be available the 9.0.7 that contains all of this? |
BrzVlad commentedJun 13, 2025
@Emanuele9xx In about a month |
Emanuele9xx commentedJun 13, 2025
Please consider to release before July because this fix a regression that causes many crashes, I think is a big issue for many many companies... |
This PR is a backport of two low risk fixes for interpreter SSA, one observed on customer provided sample and another fix for issue observed from local testing from our test suites. In addition, this includes support for a configuration env var, to enable users to disable SSA optimizations if they run into additional issues.
More conservative approach to#116250
Customer Impact
This is currently impacting users of Maui on iOS, resulting in application crash. Multiple users reported crashes in#115991
Regression
SSA optimization for interpreter was introduced in .NET9 so it is a regression from .NET8.
Testing
Tested on sample projects where the issues occurred. Normal CI testing plus CI testing with tiering disabled to ensure these fixes don't cause regressions.
Risk
Low. The fixes are very localized.