Skip to content

Conversation

AndyAyersMS
Copy link
Member

No description provided.

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jan 29, 2025
@dotnet-policy-service
Copy link
Contributor

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

@AndyAyersMS
Copy link
Member Author

@jakobbotsch PTAL
cc @dotnet/jit-contrib

New features aren't yet implemented, but that shouldn't be too hard once we settle on an API surface. I kept the interface to the rest of the code the same for now.

@jakobbotsch
Copy link
Member

jakobbotsch commented Jan 29, 2025

Sorry to keep nagging... I was hoping to see an API that can be used roughly in the following way:

ClassLayoutBuilder builder(compiler);
builder.SetSize(124);
builder.SetName("MyLayout");
builder.SetShortName("ShortName");
builder.SetPointer(8, TYPE_GC_REF);
builder.SetPointer(16, TYPE_GC_BYREF);
ClassLayout* layout = compiler->typGetCustomLayout(builder);

@EgorBo
Copy link
Member

EgorBo commented Jan 29, 2025

Sorry to keep nagging... I was hoping to see an API that can be used roughly in the following way:

ClassLayoutBuilder builder(compiler);
builder.SetSize(124);
builder.SetName("MyLayout");
builder.SetShortName("ShortName");
builder.SetPointer(8, TYPE_GC_REF);
builder.SetPointer(16, TYPE_GC_BYREF);
ClassLayout* layout = compiler->typGetCustomLayout(builder);

I'm just curious - do we need this builder for something other than Object-on-stack? Presumably hiding custom layout creation under named APIs sort of keeps VM impl details in one place since JIT has to make assumptions about object layout without asking VM

@jakobbotsch
Copy link
Member

Sorry to keep nagging... I was hoping to see an API that can be used roughly in the following way:

ClassLayoutBuilder builder(compiler);
builder.SetSize(124);
builder.SetName("MyLayout");
builder.SetShortName("ShortName");
builder.SetPointer(8, TYPE_GC_REF);
builder.SetPointer(16, TYPE_GC_BYREF);
ClassLayout* layout = compiler->typGetCustomLayout(builder);

I'm just curious - do we need this builder for something other than Object-on-stack?

One thing I've had in mind with this work for a long time is the ability to represent remainders in physical promotion. It's one of my items in #110642 and #103362.

Presumably hiding custom layout creation under named APIs sort of keeps VM impl details in one place since JIT has to make assumptions about object layout without asking VM

I am fine with adding some named helpers too (even as the static members that @AndyAyersMS started with)... I was just hoping to have a more flexible central builder type. Doing this is something I've been thinking about for a long time, but I didn't really communicate my thoughts clearly enough here.

@AndyAyersMS
Copy link
Member Author

Seems like if you want that kind of fluent pattern you'll have a bunch of intermediate state that you'll have to store somewhere. If you force the size to be disclosed up front you can allocate a ClassLayout immediately and a GC byte array if/when needed and there's no need to save up anything...

@jakobbotsch
Copy link
Member

Seems like if you want that kind of fluent pattern you'll have a bunch of intermediate state that you'll have to store somewhere. If you force the size to be disclosed up front you can allocate a ClassLayout immediately and a GC byte array if/when needed and there's no need to save up anything...

That's a good point -- it seems reasonable to require the size up front. I tried that out with the kind of shape I was thinking here: https://github.com/dotnet/runtime/compare/main...jakobbotsch:class-layout-builder?expand=1
It indeed requires saving up intermediate state, but on the flip side it keeps ClassLayout immutable and means we can't end up with a ClassLayout in a half-constructed state. What do you think?

@AndyAyersMS
Copy link
Member Author

What do you think?

Looks good to me.

jakobbotsch added a commit that referenced this pull request Feb 5, 2025
…112064)

- Add a `ClassLayoutBuilder` that can be used to build new `ClassLayout` instances with arbitrary GC pointers
- Add support for GC types to `LCL_FLD` stress to test some of this new support

Subsumes #111942
Fixes #103362
@AndyAyersMS
Copy link
Member Author

Subsumed by #112064

@AndyAyersMS AndyAyersMS closed this Feb 5, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Mar 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants