Skip to content

Read by-ref conversion operands as values - #581

Open
jogibear9988 wants to merge 1 commit into
dadhi:masterfrom
jogibear9988:fix/byref-conversion-values
Open

jogibear9988 wants to merge 1 commit into
dadhi:masterfrom
jogibear9988:fix/byref-conversion-values

Conversation

@jogibear9988

@jogibear9988 jogibear9988 commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Converting a ref parameter inside an object-array initializer can box a managed address rather than its value. On Windows ARM64/.NET 10, the new regression terminates the unfixed test process with AccessViolationException when reading a reference-containing struct.

Mark conversion operands as value reads. For nullable ref parameters, dereference in arithmetic/value contexts while retaining the existing address behavior for nullable member access. Two existing expressions change; no new compiler pass, closure shape, cache, or public API.

Directly based on upstream master 750e1e8ea602a7e72a90072ee80b0c1376f5b72d; one commit (eb91373a), independent of #558 and the other standalone PRs.

Correctness and validation

Two shared regression methods cover short/int/long/float/double/decimal and a struct containing a string, plus their nullable forms and null values. Each parameter is converted twice within an array initializer, and results are compared with System.

The unfixed .NET 10 process exits with AccessViolationException; after the patch, the same regression completes successfully.

  • Focused Standard and LightExpression regressions pass on .NET 8, .NET 10 and .NET Framework 4.7.2 (2 registered methods per API).
  • Full Release solution build passes with zero warnings/errors.
  • Legacy native suites pass: 1,756 cases on .NET 10 and 1,755 on .NET 8.
  • Framework runner passes 194 TestX + 1,639 legacy cases.
  • The full .NET 8/10 runner still reports the existing nullable MultiplyChecked parity failure for (1,1) and (2,1) in both APIs. Unmodified master was rebuilt and rerun alongside this branch; the complete failure logs are byte-for-byte identical. No regression assertions were skipped or weakened. Legacy suites were invoked separately because the existing TestX failure gates the normal runner.

Performance checks

Compared only with the master revision above, using already-working expressions. The broken expressions establish correctness and are not treated as a valid before/after performance baseline. Both Standard and LightExpression are measured. Framework is correctness-tested; the timings below cover .NET 8.0.28 and .NET 10.0.9 on Windows ARM64, SDK 10.0.301, Release.

Prebuilt expression trees; Light conversion outside the timed section; strict FEC compilation, no System fallback. Tiering disabled, fixed CPU affinity, warmed compilation and invocation. Two fresh processes with opposite load order, nine alternating samples per side, GC before samples and a 256 MiB no-GC region where available. Compilation below uses the longer approximately 100 ms pair-calibration run; invocation uses the approximately 24 ms run. Times are geometric means of the two process medians.

All measured, already-working controls retain identical decoded emitted IL and the same managed allocation per compilation/invocation. The ref-box invocation control intentionally allocates its boxed int (24 B); other invocation controls allocate zero. No persistent compiler metadata, extra collection pass, or closure allocation is added by this fix.

Timing remains noisy, including the slower rows below and variation when comparing master with itself. These measurements do not prove zero CPU regression across inputs/machines; no speedup or universal performance-equivalence claim is made.

Runtime API Control Compile ns, master → fix Change Invoke ns, master → fix Compile B/op, both Invoke B/op, both
net10.0 Standard add_control 1140.5 → 1230.8 +7.9% 1.95 → 1.85 872 0
net10.0 Standard reference_constant 941.2 → 996.4 +5.9% 1.86 → 1.95 984 0
net10.0 Standard convert_int64 1187.1 → 1168.4 -1.6% 2.10 → 2.00 872 0
net10.0 Standard ref_box_root 1190.4 → 1199.1 +0.7% 3.92 → 3.86 896 24
net10.0 Light add_control 1021.7 → 1101.9 +7.9% 1.96 → 1.95 872 0
net10.0 Light reference_constant 1243.2 → 1330.9 +7.1% 1.99 → 2.04 984 0
net10.0 Light convert_int64 1004.4 → 1000.9 -0.4% 2.05 → 2.02 872 0
net10.0 Light ref_box_root 1085.7 → 1066.2 -1.8% 4.54 → 4.16 896 24
net8.0 Standard add_control 1165.5 → 1156.1 -0.8% 1.81 → 1.83 872 0
net8.0 Standard reference_constant 964.2 → 1075.1 +11.5% 1.91 → 1.86 984 0
net8.0 Standard convert_int64 1072.5 → 1079.3 +0.6% 2.24 → 2.19 872 0
net8.0 Standard ref_box_root 1247.5 → 1153.8 -7.5% 3.49 → 3.54 896 24
net8.0 Light add_control 1045.6 → 1041.5 -0.4% 1.84 → 1.86 872 0
net8.0 Light reference_constant 937.5 → 950.2 +1.4% 1.90 → 1.85 984 0
net8.0 Light convert_int64 955.6 → 1046.0 +9.5% 2.28 → 2.22 872 0
net8.0 Light ref_box_root 1054.8 → 1045.4 -0.9% 3.67 → 3.61 896 24

The initial short run included all 16 Standard/Light controls per runtime: add, reference constant, Int32→Int64 conversion, root ref boxing, built-in Not, method-based parameter Not, a positive-origin Int32 table switch, and a two-case Int64 comparison switch. IL and allocation checks passed across all of them. The longer run focuses on the affected operation family plus add/constant controls; all short-run timing ranges, including unselected controls, are summarized below.

Runtime Initial compile range Initial compile geometric mean change Longer master/master control range Longer master/master geometric mean change
net10.0 -17.1% to +21.0% +1.7% -5.3% to +5.8% +1.4%
net8.0 -10.5% to +2.5% -2.9% -5.4% to +9.7% +1.3%

Mark conversion operands as value reads and dereference nullable ref parameters in arithmetic/value contexts while preserving nullable member-access addresses. This prevents boxing managed addresses in array initializers.

Standard and Light regressions cover primitive, reference-containing struct and nullable ref parameters, including null nullable values. Focused tests pass on net8.0, net10.0 and net472. The unfixed regression terminates the net10.0 process with AccessViolationException. Full solution builds, legacy suites and Framework runners pass. Existing master MultiplyChecked parity failures reproduce identically on net8.0/net10.0.
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