Fix Wasm SCC profile weights for #133120

Project: dotnet/runtime | Language: C# | Concepts: algorithmic-optimization, testing-strategy | Difficulty: senior
Link: https://github.com/dotnet/runtime/pull/133132

The problem

In the .NET runtime's WebAssembly (Wasm) JIT, a control flow transformation (SCC switch-dispatch) was routing all entry flow through a try block header, but failed to update the profile weight of that header. This led to inconsistent profile weights and triggered assertions during ReadyToRun/PGO (profile-guided optimization) compilation, especially when using CrossGen2.

Why it's non-trivial

Naively rewiring control flow without updating profile weights can break downstream optimizations and analyses that depend on accurate profiling data. The try block header's profile weight must reflect the sum of all entry flows after the transformation, or the compiler's assumptions about execution frequency will be invalid, potentially causing assertion failures or incorrect code generation. Additionally, reproducing and testing this scenario requires a precise control flow shape, including async/exception handling.

The approach

1. Update the try header's profile weight after rewiring control flow:

+            // All entry flow now passes through the try header before reaching the dispatcher.
+            if (tryHeader != nullptr)
+            {
+                tryHeader->setBBProfileWeight(TotalEntryWeight());
+            }

This snippet ensures that after all entry flow is routed through the try header, its profile weight is set to the aggregate entry weight. This keeps profile data consistent with the new control flow.

2. Add a regression test for the async/catch control flow shape:

A new test is added to reproduce the problematic control flow and verify the fix:

+public class Runtime_133120
+{
+    private static readonly Dictionary<int, Func<CancellationToken, Task>> s_validators = new()
+    {
+        [0] = static _ => Task.CompletedTask,
+    };
+
+    [Fact]
+    public static void TestEntryPoint()
+    {
+        ValidateAsync().GetAwaiter().GetResult();
+    }
+
+    private static async Task ValidateAsync(CancellationToken cancellationToken = default)
+    {
+        List<Exception>? exceptions = null;
+
+        foreach (Func<CancellationToken, Task> validator in s_validators.Values)
+        {
+            try
+            {
+                await validator(cancellationToken).ConfigureAwait(false);
+            }
+            catch (InvalidOperationException ex)
+            {
+                exceptions ??= new();
+                exceptions.Add(ex);
+            }
+            catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
+            {
+                throw;
+            }
+            catch (Exception ex)
+            {
+                (exceptions ??= new()).Add(ex);
+                break;
+            }
+        }
+
+        if (exceptions is not null)
+        {
+            if (exceptions.Count == 1)
+            {
+                ExceptionDispatchInfo.Capture(exceptions[0]).Throw();
+            }
+
+            if (exceptions.Count > 1)
+            {
+                throw new AggregateException(exceptions);
+            }
+        }
+    }
+}

This test exercises async code with multiple catch blocks, mirroring the problematic pattern that previously triggered the bug in the JIT.

3. Ensure the test is configured to run with CrossGen2 and the right options for browser/wasm:

+  <PropertyGroup>
+    <Optimize>true</Optimize>
+    <AlwaysUseCrossGen2 Condition="'$(TargetOS)' == 'browser'">true</AlwaysUseCrossGen2>
+  </PropertyGroup>
+  ...
+  <PropertyGroup Condition="'$(TargetOS)' == 'browser'">
+    <CrossGen2TestExtraArguments>$(CrossGen2TestExtraArguments) --codegenopt:JitSynthesizeCounts=1</CrossGen2TestExtraArguments>
+  </PropertyGroup>

These project settings ensure the test runs with CrossGen2 and synthesized profile counts on browser/wasm, matching the environment where the bug occurs.

From the review thread

Reviewer (jkotas):

Does this need RequiresProcessIsolation?

Author (AndyAyersMS):

I don't think this test needs RequiresProcessIsolation. It does not set any CLRTestEnvironmentVariable values, use native test assets, or require a test-specific corerun; the process-isolation property is used for those cases. The browser-specific AlwaysUseCrossGen2 setting is the same pattern used by Runtime_131285 and Runtime_131373, which also do not require process isolation.

Reviewer (jkotas):

I assume that we are running or soon going to run all runtime tests with wasm R2R so we are going to get regression coverage anyway.

It may be better to drop AlwaysUseCrossGen2 unless the test fails without that. Doing that improves test coverage since we will run the same test both with and without crossgen. AlwaysUseCrossGen2 forces the test to run with crossgen only.

Takeaways

• When rewriting control flow, always update profile weights to reflect the new execution paths; stale profile data can break downstream optimizations or cause assertions.
• Regression tests should closely mimic the problematic control flow and environment to ensure bugs are reliably reproduced and fixed.
• Test configuration should balance targeted coverage (e.g., forcing CrossGen2) with broader coverage (allowing tests to run in multiple modes) unless the bug only manifests in a specific configuration.