JIT: Reuse enregistered FP/SIMD/mask constants across non-last uses

[tannergooding]

LSRA previously could only reuse an already-enregistered constant when the earlier occurrence was at its last use, forcing redundant rematerialization (e.g. repeated xorps) of identical CNS_DBL/CNS_VEC/CNS_MSK values.

Change

• At build time, coalesce overlapping identical FP/SIMD/mask constants (still pending in the defList) into a single interval, adding later occurrences as redefinitions.
• At allocation time, elide a redundant redef via SetReuseRegVal when the register already holds the value. Clear the prior use''s lastUse so the coalesced interval remains one continuous range, and never restore a constant previousInterval whose register was borrowed and overwritten (it must be rematerialized instead).

These constants have no GC liveness considerations, so coalescing is safe.

Fixes #70182

Note

This pull request was authored with the assistance of GitHub Copilot.

[Copilot]

Pull request overview

This PR enhances CoreCLR JIT’s LSRA handling of floating-point/SIMD/mask constants so identical constants that are simultaneously live can share a single interval/register, reducing redundant rematerialization. It also adds a targeted JIT regression test covering several representative patterns.

Changes:

• Coalesce overlapping identical GT_CNS_DBL / GT_CNS_VEC / GT_CNS_MSK definitions into a shared constant interval during LSRA build.
• During allocation, mark coalesced constant redefinitions as ReuseRegVal when the register already contains the constant; also prevent restoring a previousInterval when it is a constant.
• Add an xUnit regression test (Runtime_70182) and wire it into Regression_o_1.csproj.

Show a summary per file

File	Description
src/coreclr/jit/lsrabuild.cpp	Adds constant-node equality + build-time coalescing; updates lastUse behavior for constant intervals.
src/coreclr/jit/lsra.cpp	Prevents restoring constant previousInterval; redefinition reuse when constant already materialized.
src/coreclr/jit/lsra.h	Declares new LSRA build helpers for constant reuse/coalescing.
src/tests/JIT/Regression/Regression_o_1.csproj	Includes the new regression test source file.
src/tests/JIT/Regression/JitBlue/Runtime_70182/Runtime_70182.cs	New xUnit regression coverage for overlapping identical constants (double/SIMD/mask scenarios).

Copilot's findings

• Files reviewed: 5/5 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

• Files reviewed: 5/5 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 5/5 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

• Files reviewed: 5/5 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 5/5 changed files
• Comments generated: 0 new

[Copilot]

Copilot's findings

• Files reviewed: 5/5 changed files
• Comments generated: 7

[tannergooding]

This basically has no diffs until after the rematerialization pr (130313) goes in, which will allow some of the pessimization to be avoided.

[JulieLeeMSFT]

LSRA changes. @jakobbotsch, PTAL.

[JulieLeeMSFT]

LGTM.

[ACK] lsra.cpp:4154–4160 — The guard preventing restoration of a constant interval whose register was borrowed is correct.

[ACK] lsrabuild.cpp:489–493 — Clearing lastUse on a prior use when a new reference is added to a coalesced constant interval is correct.

[ACK] lsrabuild.cpp:3161–3238 — The "same consumer" conflict detection (lines 3205–3229) correctly handles the multi-register hazard: if two constants feed the same FIELD_LIST (e.g. a multi-reg call arg), coalescing them would force the single resulting interval to satisfy multiple uses at one location, each potentially requiring a distinct fixed register.

[ACK] lsrabuild.cpp:3108–3135 — The constantConsumerReusesOperandReg check correctly identifies read-modify-write operations (HW intrinsics via isRMWHWIntrinsic, and non-VEX x86 FP ops via isRMWRegOper) and declines coalescing when the consumer would clobber the shared register at the point of use.

🤖 AI-generated code review

[tannergooding]

This was essentially trying to resolve the issue by explicitly identifying and merging intervals where applicable, but its tricky and because its LSRA almost any change even just limiting it to optimized code only has high TP cost.

The new pass for constant reuse that Arm is attempting to add in #128844 may actually be a better overall approach and then we could potentially remove this special constant reuse tracking in LSRA

[github-actions]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "ef52d3466ca9e7d9b9bebe52f312fc13a7dd0344",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "f00ccfddcd92f805d2d721ccaabfad6a9089e845",
  "last_reviewed_commit": "ef52d3466ca9e7d9b9bebe52f312fc13a7dd0344",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "f00ccfddcd92f805d2d721ccaabfad6a9089e845",
  "last_recorded_worker_run_id": "29680708116",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "ef52d3466ca9e7d9b9bebe52f312fc13a7dd0344",
      "review_id": 4730566878
    }
  ]
}

[github-actions]

Holistic Review

Motivation: Justified and backed by a concrete issue (#70182): LSRA could only reuse an already-enregistered FP/SIMD/mask constant when the prior occurrence was its last use, forcing repeated rematerialization (e.g. several xorps for Zero) when 4-5 registers get initialized to the same value in code like Matrix4x4.Decompose. This is a real, measurable code-quality gap.

Approach: Sound and appropriately conservative. At build time, identical FP/SIMD/mask constants still pending in defList are coalesced into one interval (later occurrences become redefinitions), with several safety guards: integer constants are excluded, same-consumer constants are rejected (a single interval can't satisfy two fixed-register uses at one location), read-modify-write consumers are declined (the shared reg would be clobbered at point of use), and partial-callee-save types are skipped. At allocation time a redundant redef is elided via SetReuseRegVal, lastUse on the coalesced use is cleared to keep the interval one continuous range, and a borrowed/overwritten constant previousInterval is no longer restored. The spill-accounting assert relaxations are correctly reasoned and documented.

Summary: ✅ LGTM. The change is well-scoped, the correctness reasoning for each guard and each relaxed spill assert is documented inline, GC liveness is correctly noted as a non-concern for these constant kinds, and it carries a meaningful regression test exercising the real vector-math shapes that drive the path. Already approved by area maintainer @JulieLeeMSFT. My independent read found no blocking issues; the notes below are confirmations and one observation for the author/human reviewer to confirm.

---

Detailed Findings

✅ Coalescing safety guards — conservative and well-reasoned

getConstantIntervalForReuse rejects coalescing whenever any matching pending def shares the consumer, has an undeterminable consumer, or feeds a read-modify-write consumer, and skips partial-callee-save types. The same-consumer rejection (a single interval cannot occupy multiple fixed registers at one location, e.g. FIELD_LIST operands) and the RMW rejection (shared reg clobbered at point of use while later refs still expect the value, unsafe because these constants are store-spilled rather than rematerialized) are both correct. The lazy computation of treeConsumer is a reasonable micro-optimization given most defs won't match.

✅ Spill accounting relaxations — correctly bounded

The two updateMaxSpill assert relaxations (reload && spillAfter for a coalesced constant, and a constant use with no assigned register reading from the read-only data section) are each gated on interval->isConstant, so non-constant intervals retain the original strict assert(currentSpill[type] > 0); currentSpill[type]--; behavior. The reasoning that a constant use reads from rodata (not a spill temp) and that a coalesced reload+respill leaves the concurrent spill count unchanged is consistent with the coalescing model.

✅ Test quality — targeted and self-validating

Runtime_70182.cs drives the exact shapes that exercise the path (Vector3/Vector4 Normalize, Quaternion.Lerp, Matrix4x4.Decompose), keeps inputs in non-readonly statics so the JIT can't constant-fold, and validates against an independent scalar reference with a tolerance — so a bad register reuse (which yields garbage, not a small rounding delta) is caught. Correctly placed in JitBlue and registered in Regression_o_1.csproj. Because AVX-512 masking makes the constants register-resident, note that coverage of the register-resident path depends on the CI hardware exercising it — which the broad runtime test matrix should provide.

💡 canRestorePreviousInterval now excludes all constant intervals — confirm intent

The new guard returns false for any previousInterval->isConstant, not only the coalesced FP/SIMD/mask kind. Integer constant intervals are never coalesced, so this broadens the restore-suppression to a class of intervals that were previously restorable. This is very likely intentional (a borrowed constant register was overwritten and must be rematerialized regardless of constant kind, which is always cheap for the constants LSRA marks isConstant), and it looks correct, but it is worth an explicit confirmation from the author that suppressing restore for integer constants here has no throughput/CQ downside in the non-coalesced path.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 142.7 AIC · ⌖ 10.7 AIC · ⊞ 10K