JIT: enhance escape analysis to understand local struct fields #113141

AndyAyersMS · 2025-03-04T22:37:30Z

Implement a very simplistic "field sensitive" analysis for gc structs where we model each struct as simply its gc field(s).

That is, given a gc struct G with GC field f, we model

G.f = ...

as an assignment to G and

   = G.f

as a read from G. Since we know G itself cannot escape, "escaping" of G means G.f can escape.

Note this conflates the fields of G: either they all escape or none of them do. We could make this more granular but it's not clear that doing so is worthwhile, and it requires more up-front work to determine how to track each field's status.

If the struct or struct fields are only consumed locally, we may be able to prove the gc referents don't escape.

This is a subset/elaboration of #112543 that does not try and reason interprocedurally.

Contributes to #104936 / #108913
Fixes #113236 (once the right inlines happen)

Implement a very simplistic "field sensitive" analysis for `Span<T>` where we model the span as simply its byref field. If the span is only consumed locally, and the array does not otherwise escape, then the array does not escape. This is a subset of dotnet#112543 that does not try and reason interprocedurally. Contributes to dotnet#104936 / dotnet#108913

PR Overview

This PR adds new tests to the JIT for enhanced escape analysis concerning Span capture. It includes the introduction of a ref struct (SpanKeeper) for capturing spans and several new test cases and helper methods that exercise various span capture patterns.

Introduces a new ref struct (SpanKeeper) to model captured spans.
Adds new test methods (e.g., SpanCaptureArray1, SpanCaptureArrayT, SpanEscapeArrayOutParam2) for Span capture scenarios.
Includes supporting helper methods (e.g., Use(Span), TrashStack) to drive the tests.

Reviewed Changes

File	Description
src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs	Added tests and helper methods to verify proper handling of Span captures in varying contexts

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs:519

The output parameter 'b' in SpanEscapeArrayOutParam2Helper is used before it is explicitly assigned. Consider initializing 'b' (e.g., using 'b = default;') before setting its fields to ensure proper assignment.

b.span = x;

AndyAyersMS · 2025-03-04T22:40:12Z

@EgorBo @jakobbotsch PTAL
cc @dotnet/jit-contrib

Note for full-blown field-sensitive handling of structs we would need to build connection graph edges when fields are loaded; since we know loads from spans can't cause escapes we bypass that for now.

jakobbotsch · 2025-03-04T22:41:35Z

src/coreclr/jit/objectalloc.cpp

+                        newLayout = m_compiler->typGetBlkLayout(layout->GetSize());
+                        JITDUMP("Changing layout of span V%02u to block\n", lclNum);
+                        lclVarDsc->ChangeLayout(newLayout);


I think for this we want a layout with proper padding information, otherwise the promotion will be suboptimal compared to the original layout.

Ah, ok... maybe an "erase GC" method on a layout?

Is it even necessary for us to change the type here? Seems like the original byref type should be ok to now point to a stack-allocated thing.

Ah, ok... maybe an "erase GC" method on a layout?

I think you can do something like:

ClassLayoutBuilder builder(lcl->GetLayout()->GetSize()); builder.CopyInfoFrom(0, lcl->GetLayout(), /* copyPadding */ true); builder.SetGCPtrType(OFFSETOF__CORINFO_Span__reference / TARGET_POINTER_SIZE, TYP_I_IMPL);

Although I suppose if we generalized this PR to arbitrary structs we would indeed want something that only copies the padding information, so maybe introducing another parameter to CopyInfoFrom to ignore the GC information would be the way to go.

jakobbotsch · 2025-03-04T22:47:03Z

Note for full-blown field-sensitive handling of structs we would need to build connection graph edges when fields are loaded; since we know loads from spans can't cause escapes we bypass that for now.

Maybe I misunderstand this, but what about e.g. MemoryMarshal.GetReference API? Or even ref span[0]?

Can the handling of this PR be generalized to treat any struct with references in it as one big reference that causes unification to happen between everything that read/wrote the struct?

AndyAyersMS · 2025-03-05T00:10:22Z

Note for full-blown field-sensitive handling of structs we would need to build connection graph edges when fields are loaded; since we know loads from spans can't cause escapes we bypass that for now.

Maybe I misunderstand this, but what about e.g. MemoryMarshal.GetReference API? Or even ref span[0]?

Can the handling of this PR be generalized to treat any struct with references in it as one big reference that causes unification to happen between everything that read/wrote the struct?

Indeed, looks like we need to do some modelling of the loads. I think we can just conflate the struct with its fields. Let me see how this works out.

We conflate a struct with its fields, since we know the struct itself can't escape. We rely on accesses not being able to walk from one local to another. We try to be properly conservative around byrefs.

jakobbotsch · 2025-03-07T20:24:07Z

src/coreclr/jit/layout.cpp

+            // Allow a source GC_REF to match a custom GC_BYREF
+            //
+            if ((layout2->GetGCPtrType(i) == TYP_REF) && (layout1->GetGCPtrType(i) == TYP_BYREF) &&
+                layout1->IsCustomLayout())
+            {
+                continue;
+            }


Changing the definition of layout compatibility seems quite questionable to me. Why is it necessary?

GCStruct a; a.obj = ..; // doesn't escape GCStruct b; b.obj = ..; // escapes // Assignment is legal, but layouts are not compatible w/o the above // a.obj must be BYREF as it may refer to stack or heap a = b

I was tempted to retain the class handle in new layout, but didn't.

Note allowing b = a would be incompatible with the results of escape analysis, which is why this is one-sided (source can be byref when dest is ref, but not vice-versa).

So are you saying that for the

object aObj = ...; // doesn't escape object bObj = ...; // escapes aObj = bObj;

case, today we will produce a STORE_LCL_VAR that stores a TYP_REF into a TYP_BYREF?

Yes, we will retype aObj appearances to be byrefs, as it will be "possibly stack pointing". And aObj itself in the local table.

This has been there since forever.

Ok, it makes it somewhat more palatable. But it seems like we need to separate the notions of "user facing" layout compatibility, as used by e.g. Unsafe.BitCast, and "implementation-specific assignment compatibility" which is non-symmetric and where we rely on internal coreclr implementation details. As you probably know there could be a potential future where byrefs pointing at the method table would be illegal... I guess in that case we would need a third kind of GC pointer. Or we would have to switch object stack allocation to only kick in when it can reason about and change all accesses of the object.

I'm open to suggestions on how to express this better. Perhaps the presence of a custom layout for the dest might be enough?

We could get here with custom layouts on both sides:

GCStruct a; a.obj = ..; // doesn't escape, may point at heap [byref] GCStruct b; b.obj = ..; // doesn't escape, never points at heap [nongc] a = b

I would just suggest to rename this to ClassLayout::AssignmentCompatible(ClassLayout* dest, ClassLayout* src) and allow the byref <- ref assignments generally. And then switch Unsafe.BitCast to a version that keeps the stricter behavior.
Should probably be a separate PR...

(Maybe BitCast doesn't even need any changes, I was under the assumption that it threw an exception on incompatible layouts, but it does not seem like it is the case)

jakobbotsch · 2025-03-07T20:25:16Z

src/coreclr/jit/layout.cpp

+            {
+                gcType = TYPE_GC_BYREF;
+            }
+            SetGCPtr(startSlot + slot, gcType);
        }
    }
 }


This code feels very object stack allocation specific. Put it in objectalloc.cpp?

jakobbotsch · 2025-03-07T20:26:24Z

src/coreclr/jit/compiler.h

+    // Get a layout like an existing layout, with all gc refs removed
+    ClassLayout* typGetNonGCLayout(ClassLayout* existingLayout);
+    // Get a layout like an existing layout, with all gc refs changed to byrefs
+    ClassLayout* typGetByrefLayout(ClassLayout* existingLayout);


This feels like something that only stack allocation is ever going to use, so it would make more sense to me to keep it in objectalloc.h/cpp.

jakobbotsch · 2025-03-07T20:27:56Z

src/coreclr/jit/layout.cpp

+    if (prefix != nullptr)
+    {
+        char* newName      = nullptr;
+        char* newShortName = nullptr;
+
+        if (layoutName != nullptr)
+        {
+            size_t len = strlen(prefix) + strlen(layoutName) + 1;
+            newName    = m_compiler->getAllocator(CMK_DebugOnly).allocate<char>(len);
+            sprintf_s(newName, len, "%s%s", prefix, layoutShortName);
+        }
+
+        if (layoutShortName != nullptr)
+        {
+            size_t len   = strlen(prefix) + strlen(layoutName) + 1;
+            newShortName = m_compiler->getAllocator(CMK_DebugOnly).allocate<char>(len);
+            sprintf_s(newShortName, len, "%s%s", prefix, layoutShortName);
+        }
+
+        SetName(newName, newShortName);
+    }
+    else
+    {
+        SetName(layoutName, layoutShortName);
+    }


This can use printfAlloc.

AndyAyersMS · 2025-03-07T21:46:33Z

Assert failure(PID 3416 [0x00000d58], Thread: 2092 [0x082c]): Assertion failed 'unreached' in 'ManagedSequential:LayoutClassObjectBaseIsManagedSequential()' during 'Allocate Objects' (IL size 36; hash 0xc0408010; FullOpts)

    File: D:\a\_work\1\s\src\coreclr\jit\objectalloc.cpp:1460
    Image: C:\h\w\A5540979\p\corerun.exe

Wonder if this is what I was seeing locally, somehow a managed type makes it to a UDIV.

AndyAyersMS · 2025-03-08T03:09:45Z

We are trying to retype after stack allocation

Allocating V02 on the stack
...
Changing the type of V00 from ref to long
Changing the type of V02 from ref to long
...
STMT00009 ( ??? ... ??? )
               [000027] DA-X-------                         *  STORE_LCL_VAR int    V04 tmp3         
               [000015] ---X-------                         \--*  CAST      int <- long
               [000014] ---X-------                            \--*  SUB       long  
               [000011] ---X-------                               +--*  FIELD_ADDR byref  ManagedSequential+LayoutClassObjectBase:l1
               [000010] -----------                               |  \--*  LCL_VAR   ref    V00 loc0         
               [000013] -----------                               \--*  LCL_VAR   byref  V03 tmp2

and don't expect to see a cast.

Copilot bot review requested due to automatic review settings March 4, 2025 22:37

dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 4, 2025

dotnet-policy-service bot assigned AndyAyersMS Mar 4, 2025

Copilot AI reviewed Mar 4, 2025

View reviewed changes

jakobbotsch reviewed Mar 4, 2025

View reviewed changes

Generalize to handling all GC structs.

5fcfe24

We conflate a struct with its fields, since we know the struct itself can't escape. We rely on accesses not being able to walk from one local to another. We try to be properly conservative around byrefs.

jakobbotsch reviewed Mar 7, 2025

View reviewed changes

AndyAyersMS changed the title ~~JIT: enhance escape analysis to understand Span<T> capture~~ JIT: enhance escape analysis to understand local struct fields Mar 7, 2025

fix GT_SUB type propagation

7b0172a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JIT: enhance escape analysis to understand local struct fields #113141

JIT: enhance escape analysis to understand local struct fields #113141

AndyAyersMS commented Mar 4, 2025 •

edited

Loading

AndyAyersMS commented Mar 4, 2025

jakobbotsch Mar 4, 2025

AndyAyersMS Mar 4, 2025

jakobbotsch Mar 4, 2025

jakobbotsch Mar 4, 2025

jakobbotsch Mar 4, 2025

jakobbotsch commented Mar 4, 2025

AndyAyersMS commented Mar 5, 2025

jakobbotsch Mar 7, 2025

AndyAyersMS Mar 7, 2025

AndyAyersMS Mar 7, 2025

AndyAyersMS Mar 7, 2025

jakobbotsch Mar 7, 2025

AndyAyersMS Mar 7, 2025 •

edited

Loading

jakobbotsch Mar 7, 2025 •

edited

Loading

AndyAyersMS Mar 7, 2025

jakobbotsch Mar 7, 2025

jakobbotsch Mar 7, 2025

jakobbotsch Mar 7, 2025

jakobbotsch Mar 7, 2025

jakobbotsch Mar 7, 2025

AndyAyersMS commented Mar 7, 2025

AndyAyersMS commented Mar 8, 2025

JIT: enhance escape analysis to understand local struct fields #113141

Are you sure you want to change the base?

JIT: enhance escape analysis to understand local struct fields #113141

Conversation

AndyAyersMS commented Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

PR Overview

Reviewed Changes

AndyAyersMS commented Mar 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakobbotsch commented Mar 4, 2025

AndyAyersMS commented Mar 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndyAyersMS Mar 7, 2025 • edited Loading

Choose a reason for hiding this comment

jakobbotsch Mar 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndyAyersMS commented Mar 7, 2025

AndyAyersMS commented Mar 8, 2025

AndyAyersMS commented Mar 4, 2025 •

edited

Loading

AndyAyersMS Mar 7, 2025 •

edited

Loading

jakobbotsch Mar 7, 2025 •

edited

Loading