JIT: Model promoted struct definitions through visitors

[jakobbotsch]

Teach VisitLocalDefs to report the logical field definitions represented by stores to promoted struct locals, including ranged STORE_LCL_FLD and call definitions. Use typed providers so definition properties are computed only when requested, while keeping VisitLocalDefNodes for consumers that need the physical IR nodes.

Update SSA, value numbering, copy propagation, async analysis, alias tracking, loop queries, and assertion invalidation to consume the appropriate visitor. This removes promotion-specific definition expansion from those consumers and prepares the IR for representing multiple local definitions explicitly with a new STORE_LCL_VARS node.

[azure-pipelines]

Azure Pipelines:
Successfully started running 5 pipeline(s).
11 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[azure-pipelines]

Azure Pipelines:
Successfully started running 5 pipeline(s).
11 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

🟡 Changes recommended

LoopDefinitions no longer records logical defs for promoted struct stores (risking stale assertions across loop backedges), and there is also a debug-assert keying issue in copyprop’s shadowed-parameter early return.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR refactors how the JIT enumerates local definitions so that consumers can see logical defs for promoted struct locals (including ranged STORE_LCL_FLD and call-induced defs) via VisitLocalDefs providers, reducing promotion-specific expansion in downstream analyses and paving the way for multi-def IR forms like STORE_LCLS.

Changes:

• Add local-definition “provider” types and extend GenTree::VisitLocalDefs to report logical defs for promoted structs and call defs.
• Update multiple analyses/optimizations (VN, SSA renaming, copyprop, async analysis, alias tracking, loop queries, SSA diagnostics, liveness) to consume VisitLocalDefs vs. physical-def-node enumeration.
• Simplify or remove ad-hoc promoted-struct def expansion in various consumers.

File summaries

File	Description
src/coreclr/jit/valuenum.cpp	Switch VN assignment for local stores/call defs to consume logical defs from VisitLocalDefs.
src/coreclr/jit/ssabuilder.cpp	Update SSA renaming to set SSA numbers through definition providers and split address-exposed handling via physical def nodes.
src/coreclr/jit/sideeffects.cpp	Record local writes using logical defs (with fallback when none are reported).
src/coreclr/jit/promotionliveness.cpp	Query promoted/call def sizes via provider APIs.
src/coreclr/jit/morph.cpp	Adjust var-def flagging and assertion invalidation to operate on physical def nodes where appropriate.
src/coreclr/jit/lower.cpp	Switch parameter register-kill discovery to consume logical defs.
src/coreclr/jit/liveness.cpp	Update call-life computation to operate on physical def nodes.
src/coreclr/jit/lclmorph.cpp	Remove manual promotion-specific expansion from loop definition tracking (now needs to be replaced by logical-def visitation).
src/coreclr/jit/gentree.h	Add declarations for IsEntireLocalDef and new local-def visitor helpers.
src/coreclr/jit/gentree.cpp	Update local-store detection to rely on logical def enumeration.
src/coreclr/jit/flowgraph.cpp	Update loop def visitors/queries to consume logical defs and adjust semantics for field/parent matching.
src/coreclr/jit/fgdiagnostic.cpp	Update SSA checking to validate logical defs and their use-def relationships.
src/coreclr/jit/copyprop.cpp	Refactor copyprop def-stack tracking to push per logical def (lclNum, ssaNum) rather than expanding composite SSA names manually.
src/coreclr/jit/compiler.hpp	Introduce LocalDefProvider + concrete provider types and implement promoted-range def enumeration helpers.
src/coreclr/jit/compiler.h	Update fgValueNumberLocalStore and optCopyPropPushDef declarations to match new usage.
src/coreclr/jit/asyncanalysis.cpp	Mark mutated locals via logical defs for stores/calls.
src/coreclr/jit/async.cpp	Update async live-set construction to use provider semantics for “entire” defs.

Review details

• Files reviewed: 17/17 changed files
• Comments generated: 2
• Review effort level: Lite

[jakobbotsch]

cc @dotnet/jit-contrib PTAL @AndyAyersMS

No diffs, minor TP changes. I spent a while on TP, the current shape I found where the visitor functors use const auto& def and we have separate classes that compute various properties lazily was the best one.

This is an intermediate step towards adding a new node that stores multiple locals simultaneously from a single source. With this PR VisitLocalDefs starts to return multiple definitions for a STORE_LCL_VAR node that targets the promoted parent local, and similarly for CALL retbuffer definitions (and STORE_LCL_FLD that overlaps multiple fields) when they target promoted parent locals.

After this PR I expect to add a new STORE_LCL_VARS node and teach physical promotion to produce it for stores from calls and other multireg nodes. I am also considering whether we should transform old promoted locals into this shape, but I might leave that as is since I expect to remove it anyway.

[Copilot]

🔵 Needs a closer look

It rewires multiple core JIT analyses around new definition-visitor semantics, so the regression surface is large and needs careful human validation.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/jit/compiler.h:6508

• Compiler::fgValueNumberLocalStore is now a member function template declared in compiler.h, but its definition lives only in valuenum.cpp. That works for current callers (all in valuenum.cpp), but it becomes a link-time trap if any other TU ever calls it (the template definition won’t be visible for instantiation). Consider either moving the template definition to a header (e.g., compiler.hpp) or making this a static/local helper in valuenum.cpp so it isn’t exposed via compiler.h.

• Files reviewed: 16/16 changed files
• Comments generated: 0 new
• Review effort level: Lite

[jakobbotsch]

Ping @AndyAyersMS

[AndyAyersMS]

Ping @AndyAyersMS

Can you put me down as a reviewer going forward so I don't miss seeing these?

[jakobbotsch]

Ping @AndyAyersMS

Can you put me down as a reviewer going forward so I don't miss seeing these?

Ah, whoops, I usually do that, but looks like I missed it here.

[Copilot]

🔵 Needs a closer look

This broad JIT refactor has unresolved nit findings and warrants human review.

Review details

Suppressed comments (1)

src/coreclr/jit/compiler.hpp:4458

• The new provider hierarchy replaces the old LocalDef representation, but the old LocalDef struct in gentree.h:755-769 is still present and has no remaining references. Please remove that obsolete definition so the JIT does not retain two competing local-definition models.

template <typename TDerived>
struct LocalDefProvider
{
    bool HasMultiDefIndex() const
    {
        return static_cast<const TDerived*>(this)->GetMultiDefIndex() != BAD_VAR_NUM;

• Files reviewed: 16/16 changed files
• Comments generated: 1
• Review effort level: Lite

[AndyAyersMS]

Seems like a very nice generalization.

Nits:

• maybe distinguish the two kinds of def visitors better, seems easy to confuse VisitLocalDefs and VisitLocalDefNodes?
• remove dead LocalDef in gentree.h
• template function declared globally but defined in a single compilation unit is a bit odd (fgValueNumberLocalStore)

[jakobbotsch]

Seems like a very nice generalization.

Nits:

• maybe distinguish the two kinds of def visitors better, seems easy to confuse VisitLocalDefs and VisitLocalDefNodes?

I renamed VisitLocalDefs to VisitLogicalLocalDefs and VisitLocalDefNodes to VisitPhysicalLocalDefNodes

• remove dead LocalDef in gentree.h

Deleted

• template function declared globally but defined in a single compilation unit is a bit odd (fgValueNumberLocalStore)

I kept this as is, not sure if any other option would be cleaner (I like having the declaration together with the rest of the declarations in compiler.h).

[Copilot]

🟡 Changes recommended

Two unresolved findings remain, including one critical value-numbering issue and one moderate local-store matching issue.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

src/coreclr/jit/gentree.cpp:8141

• This callback now sees only logical field definitions, so it no longer matches the physical promoted parent local. For example, a STORE_LCL_VAR or return-buffer definition whose physical node targets promoted V is reported as V's fields; querying gtTreeHasLocalStore(tree, V) therefore returns false even though the store affects V. The helper is used by loop initialization and if-conversion, and gtTreeHasLocalRead still explicitly treats parent/field accesses as aliases. Include the physical definition's local in this match (or otherwise retain the promoted-parent check).

• Files reviewed: 18/18 changed files
• Comments generated: 1
• Review effort level: Lite

[jakobbotsch]

@AndyAyersMS Can you give this another approval? Had to merge due to a conflict

[Copilot]

Copilot review overview

🟡 Changes recommended

Preserve the promoted physical parent in m_lclVarWrites to avoid incorrect alias analysis.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Preserve promoted parent in local-write tracking · New
• The debug assertion in the shadowed-parameter early-return is checking the wrong key. curSsaName…

Resolved since last review (1)

• VisitLogicalLocalDefs calls this lambda once per logical field, so a promoted return-buffer (or…