Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: initial support for stack allocating arrays of GC type #112250

Merged
merged 1 commit into from
Feb 9, 2025
+97 −27
Merged

JIT: initial support for stack allocating arrays of GC type #112250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11118,6 +11118,10 @@ class Compiler
ClassLayout* typGetBlkLayout(unsigned blockSize);
// Get the number of a layout having the specified size but no class handle.
unsigned typGetBlkLayoutNum(unsigned blockSize);
// Get the layout for the specified array of known length
ClassLayout* typGetArrayLayout(CORINFO_CLASS_HANDLE classHandle, unsigned length);
// Get the number of a layout for the specified array of known length
unsigned typGetArrayLayoutNum(CORINFO_CLASS_HANDLE classHandle, unsigned length);

var_types TypeHandleToVarType(CORINFO_CLASS_HANDLE handle, ClassLayout** pLayout = nullptr);
var_types TypeHandleToVarType(CorInfoType jitType, CORINFO_CLASS_HANDLE handle, ClassLayout** pLayout = nullptr);
Expand Down
90 changes: 88 additions & 2 deletions src/coreclr/jit/layout.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,18 @@ ClassLayout* Compiler::typGetBlkLayout(unsigned blockSize)
return typGetCustomLayout(ClassLayoutBuilder(this, blockSize));
}

unsigned Compiler::typGetArrayLayoutNum(CORINFO_CLASS_HANDLE classHandle, unsigned length)
{
ClassLayoutBuilder b = ClassLayoutBuilder::BuildArray(this, classHandle, length);
return typGetCustomLayoutNum(b);
}

ClassLayout* Compiler::typGetArrayLayout(CORINFO_CLASS_HANDLE classHandle, unsigned length)
{
ClassLayoutBuilder b = ClassLayoutBuilder::BuildArray(this, classHandle, length);
return typGetCustomLayout(b);
}

//------------------------------------------------------------------------
// Create: Create a ClassLayout from an EE side class handle.
//
Expand Down Expand Up @@ -717,8 +729,8 @@ bool ClassLayout::AreCompatible(const ClassLayout* layout1, const ClassLayout* l
}

//------------------------------------------------------------------------
// ClassLayoutbuilder: Construct a new builder for a class layout of the
// specified size, with all data being considered as non-padding.
// ClassLayoutBuilder: Construct a new builder for a class layout of the
// specified size.
//
// Arguments:
// compiler - Compiler instance
Expand All @@ -730,6 +742,80 @@ ClassLayoutBuilder::ClassLayoutBuilder(Compiler* compiler, unsigned size)
{
}

//------------------------------------------------------------------------
// BuildArray: Construct a builder for an array layout
//
// Arguments:
// compiler - Compiler instance
// arrayHandle - class handle for array
// length - array length (in elements)
//
// Note:
// For arrays of structs we currently do not copy any struct padding,
// with the presumption that it is unlikely we will ever promote array elements.
//
ClassLayoutBuilder ClassLayoutBuilder::BuildArray(Compiler* compiler, CORINFO_CLASS_HANDLE arrayHandle, unsigned length)
{
assert(length <= CORINFO_Array_MaxLength);
assert(arrayHandle != NO_CLASS_HANDLE);

CORINFO_CLASS_HANDLE elemClsHnd = NO_CLASS_HANDLE;
CorInfoType corType = compiler->info.compCompHnd->getChildType(arrayHandle, &elemClsHnd);
var_types type = JITtype2varType(corType);

ClassLayout* elementLayout = nullptr;
unsigned elementSize = 0;

if (type == TYP_STRUCT)
{
elementLayout = compiler->typGetObjLayout(elemClsHnd);
elementSize = elementLayout->GetSize();
}
else
{
elementSize = genTypeSize(type);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose if we really wanted to we could provide a constructor for a class layout for primitive types, which could be used to unify paths like these.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like that might have been the original intention since we bias the indexes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case we can just create them as custom layouts now, but I suppose it would be slightly less efficient than this approach.

}

ClrSafeInt<unsigned> totalSize(elementSize);
totalSize *= static_cast<unsigned>(length);
totalSize += static_cast<unsigned>(OFFSETOF__CORINFO_Array__data);
assert(!totalSize.IsOverflow());

ClassLayoutBuilder builder(compiler, totalSize.Value());

if (elementLayout != nullptr)
{
if (elementLayout->HasGCPtr())
{
unsigned offset = OFFSETOF__CORINFO_Array__data;
for (unsigned i = 0; i < length; i++)
{
builder.CopyInfoFrom(offset, elementLayout, /* copy padding */ false);
offset += elementSize;
}
}
}
else if (varTypeIsGC(type))
{
unsigned offset = OFFSETOF__CORINFO_Array__data;
for (unsigned i = 0; i < length; i++)
{
assert((offset % TARGET_POINTER_SIZE) == 0);
unsigned const slot = offset / TARGET_POINTER_SIZE;
builder.SetGCPtrType(slot, type);
offset += elementSize;
}
}

#ifdef DEBUG
const char* className = compiler->eeGetClassName(arrayHandle);
const char* shortClassName = compiler->eeGetShortClassName(arrayHandle);
builder.SetName(className, shortClassName);
#endif

return builder;
}

//------------------------------------------------------------------------
// GetOrCreateGCPtrs: Get or create the array indicating GC pointer types.
//
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/layout.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ class ClassLayoutBuilder
#ifdef DEBUG
void SetName(const char* name, const char* shortName);
#endif

static ClassLayoutBuilder BuildArray(Compiler* compiler, CORINFO_CLASS_HANDLE arrayType, unsigned length);
};

// Encapsulates layout information about a class (typically a value class but this can also be
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/objectalloc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,7 @@ unsigned int ObjectAllocator::MorphNewArrNodeIntoStackAlloc(GenTreeCall*
blockSize = AlignUp(blockSize, 8);
}

comp->lvaSetStruct(lclNum, comp->typGetBlkLayout(blockSize), /* unsafeValueClsCheck */ false);
comp->lvaSetStruct(lclNum, comp->typGetArrayLayout(clsHnd, length), /* unsafe */ false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to propagate the layout through instead of blockSize? We could probably save a call to getChildType.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I should get rid of the redundant layout fetch.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only a small step from there to using layouts everywhere.

I think I should do that first (leaving GC arrays disabled) and verify zero diff, then come back to this and enable gc arrays.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After poking at it some, I think it might be simpler to take this change first and then revise the entire thing to be more layout centric (getting rid of the special box type, etc).

Thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds fine to me.

lclDsc->lvStackAllocatedObject = true;

// Initialize the object memory if necessary.
Expand Down
26 changes: 2 additions & 24 deletions src/coreclr/jit/objectalloc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,30 +359,8 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNu
return false;
}

CORINFO_CLASS_HANDLE elemClsHnd = NO_CLASS_HANDLE;
CorInfoType corType = comp->info.compCompHnd->getChildType(clsHnd, &elemClsHnd);
var_types type = JITtype2varType(corType);
ClassLayout* elemLayout = type == TYP_STRUCT ? comp->typGetObjLayout(elemClsHnd) : nullptr;

if (varTypeIsGC(type) || ((elemLayout != nullptr) && elemLayout->HasGCPtr()))
{
*reason = "[array contains gc refs]";
return false;
}

const unsigned elemSize = elemLayout != nullptr ? elemLayout->GetSize() : genTypeSize(type);

ClrSafeInt<unsigned> totalSize(elemSize);
totalSize *= static_cast<unsigned>(length);
totalSize += static_cast<unsigned>(OFFSETOF__CORINFO_Array__data);

if (totalSize.IsOverflow())
{
*reason = "[overflow array length]";
return false;
}

classSize = totalSize.Value();
ClassLayout* const layout = comp->typGetArrayLayout(clsHnd, (unsigned)length);
classSize = layout->GetSize();
}
else if (allocType == OAT_NEWOBJ)
{
Expand Down
Loading