Skip to content

Fix duplicate intermediate addresses in nested struct assignments - #585

Open
jogibear9988 wants to merge 1 commit into
dadhi:masterfrom
jogibear9988:fix/nested-struct-field-address
Open

jogibear9988 wants to merge 1 commit into
dadhi:masterfrom
jogibear9988:fix/nested-struct-field-address

Conversation

@jogibear9988

@jogibear9988 jogibear9988 commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Assignments through nested struct fields can leave an extra managed address on the evaluation stack. On master, the regression compiles strictly but invoking the delegate throws InvalidProgramException in both expression APIs.

Compute the duplication destination once in the existing member emitter: duplicate the receiver or the final writable address when requested, and retain one address for intermediate struct owners. No new emitter, traversal, context metadata or allocation is introduced.

Validation

Covers value/void assignments, one-time receiver/RHS evaluation, compound and increment results, and ref access. For the nested compound shape where System lowers through a struct copy, the test records that behavior separately and checks writable storage using an explicit once-only assignment expansion.

  • The unfixed master regression fails in both APIs; the same fixture passes after the patch on .NET 8, .NET 10 and .NET Framework 4.7.2. Strict FEC compilation is used.
  • Full Release solution build passes with zero warnings/errors.
  • Legacy suites pass: 1,756 on .NET 10 and 1,755 on .NET 8.
  • Framework runner passes 251 TestX + 1,639 legacy cases (en-US culture for existing message assertions).
  • Full modern runners retain only the upstream nullable MultiplyChecked parity failure for (1,1)/(2,1), in both APIs. Unmodified master was rebuilt and checked separately. No assertions were skipped or weakened; legacy suites ran separately because that existing TestX failure gates the normal runner.

Performance checks

Compared with master at f8f2a240abd650dda738162957606aac2659873f using already-working expressions, for both Standard and LightExpression. The broken regression establishes correctness and is not used as a before/after speed baseline. All selected working controls retain identical decoded emitted IL and the same measured compilation/invocation allocation counts.

Measurement setup and full results

Windows ARM64, SDK 10.0.301, .NET 8.0.28/10.0.9, Release. Prebuilt trees; Light conversion outside the timed section; strict CompileFast without System fallback; fixed CPU affinity, tiering disabled, 300 compilation warmups and 100,000 invocation warmups. Two fresh processes with opposite load order, nine alternating samples per side, GC before samples and a 256 MiB no-GC region where available. Table values are geometric means of process medians; compilation uses the longer approximately 100 ms paired-calibration run and invocation uses approximately 24 ms. Framework is correctness-tested, not timed.

Runtime / API / workload Compile ns, master → fix Change Compile B/op, both Invoke ns, master → fix Invoke B/op, both
net10.0 / Standard / add_control 1153.1 → 1150.4 -0.2% 872 1.92 → 1.92 0
net10.0 / Standard / flat_field_assign 1336.5 → 1314.3 -1.7% 928 1.84 → 1.83 0
net10.0 / Standard / nested_field_read 1250.0 → 1204.3 -3.7% 952 1.87 → 1.87 0
net10.0 / Light / add_control 1044.1 → 1010.2 -3.2% 872 1.89 → 1.91 0
net10.0 / Light / flat_field_assign 1167.2 → 1139.1 -2.4% 928 1.89 → 1.92 0
net10.0 / Light / nested_field_read 1047.7 → 1119.4 +6.8% 952 1.92 → 2.02 0
net8.0 / Standard / add_control 1580.6 → 1796.6 +13.7% 872 1.87 → 1.89 0
net8.0 / Standard / flat_field_assign 2106.3 → 2178.8 +3.4% 928 2.17 → 1.99 0
net8.0 / Standard / nested_field_read 1614.3 → 1423.1 -11.8% 952 1.90 → 1.90 0
net8.0 / Light / add_control 1441.2 → 1753.1 +21.6% 872 2.15 → 2.46 0
net8.0 / Light / flat_field_assign 1791.9 → 1512.7 -15.6% 928 2.38 → 2.32 0
net8.0 / Light / nested_field_read 1726.9 → 1656.9 -4.1% 952 1.89 → 1.92 0

The full short run covers add, reference constant, root/nested constructors, a 16-argument delegate, flat field assignment and nested field reads. The longer run focuses on this fix's affected working paths plus unchanged controls. Slower observations are included above. Identical-binary master/master compilation variation in the same setup was net10.0: -11.6% to +10.4%; net8.0: -21.3% to +14.0%. These timings do not prove zero overhead for every input/machine, and no speedup claim is made. Runtime timing differences above are observations of identical emitted control IL.

All-process compilation timing ranges

These ranges include the less stable observations, not just the longer-run aggregate in the table. They are individual process-median changes across working controls; they are not speedup claims.

Runtime Short-run range Longer-run range Unchanged master/master longer-run range
net10.0 -29.8% to +30.6% -8.0% to +10.9% -13.9% to +10.7%
net8.0 -38.4% to +24.3% -20.0% to +42.0% -29.5% to +25.0%

Assignments through nested struct fields can leave an extra managed address on the evaluation stack. On master, the regression compiles strictly but invoking the delegate throws InvalidProgramException in both expression APIs. Compute the duplication destination once in the existing member emitter: duplicate the receiver or the final writable address when requested, and retain one address for intermediate struct owners. No new emitter, traversal, context metadata or allocation is introduced.

Focused Standard/Light regressions pass on .NET 8, .NET 10 and .NET Framework 4.7.2. Full solution and legacy suites pass. Modern full runners retain only the reproduced upstream nullable MultiplyChecked parity failure. Working benchmark controls retain allocation counts and emitted IL.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant