[release/9.0-staging] [mono][interp] Disable SSA from default set of optimizations #116250
Conversation
There are some issues with it that need resolving
There was a problem hiding this comment.
Pull Request Overview
This PR disables the SSA optimization in the interpreter to avoid application crashes on iOS, while still allowing users to re-enable SSA via an environment variable if needed.
- Removed the SSA flag from the default interpreter optimizations.
- Added support to override interpreter options using the MONO_INTERPRETER_OPTIONS environment variable.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/mono/mono/mini/interp/interp.h | Removed INTERP_OPT_SSA from the default set of optimizations. |
| src/mono/mono/mini/interp/interp.c | Added environment variable support to allow user customization of interpreter options. |
| @@ -43,7 +43,7 @@ enum { | |||
| #endif | |||
| INTERP_OPT_SSA = 128, | |||
| INTERP_OPT_PRECISE_GC = 256, | |||
| INTERP_OPT_DEFAULT = INTERP_OPT_INLINE | INTERP_OPT_CPROP | INTERP_OPT_SUPER_INSTRUCTIONS | INTERP_OPT_BBLOCKS | INTERP_OPT_TIERING | INTERP_OPT_SIMD | INTERP_OPT_SSA | INTERP_OPT_PRECISE_GC | |||
| INTERP_OPT_DEFAULT = INTERP_OPT_INLINE | INTERP_OPT_CPROP | INTERP_OPT_SUPER_INSTRUCTIONS | INTERP_OPT_BBLOCKS | INTERP_OPT_TIERING | INTERP_OPT_SIMD | INTERP_OPT_PRECISE_GC | |||
There was a problem hiding this comment.
Consider adding a comment here to explain that the SSA optimization flag was intentionally removed from the default settings due to stability issues on iOS.
Copilot uses AI. Check for mistakes.
| @@ -8981,6 +8981,10 @@ mono_ee_interp_init (const char *opts) | |||
| set_context (NULL); | |||
|
|
|||
| interp_parse_options (opts); | |||
|
|
|||
| const char *env_opts = g_getenv ("MONO_INTERPRETER_OPTIONS"); | |||
There was a problem hiding this comment.
[nitpick] It may be beneficial to document the behavior of parsing interpreter options from the environment variable, clarifying that these options override the defaults when provided.
Copilot uses AI. Check for mistakes.
|
Tagging subscribers to this area: @BrzVlad, @kotlarmilos |
|
cc @lewing |
|
When will be available? |
|
@SteveMCarroll Key points regarding this issue:
|
BrzVlad
force-pushed
the
backport-disable-ssa
branch
from
c968107 to
61cca41
Compare
|
Tested without changes to the optimization set and the same WASM failures appear, meaning they are not related. |
|
@BrzVlad reminder that servicing code complete for 9.0.7 is today @ 3PM PST. |
|
Closing in favor of #116428 |
Key points regarding this issue:
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 with a repro sample provided by a customer. Tested the no-ssa configuration on CI to ensure it doesn't cause other regressions.
Risk
Low. This disables the SSA optimization, which is pretty much the same configuration that we had before in .NET8. SSA disabled compilation was also currently used for methods deemed too massive to do SSA transformation from the start, so it is not a bitrotten configuration. The drawback is that SSA optimization would improve quality of compiled code by about 10% so some perf regressions would be expected. For any eventuality, a new env var support is added to the interpreter, allowing for user to select the desired optimizations, reverting to original behavior.