Repository navigation
Read by-ref conversion operands as values - #581
Open
jogibear9988 wants to merge 1 commit into
Open
jogibear9988 wants to merge 1 commit into
jogibear9988 wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
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.
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.