Skip to content

Commit

Permalink
Fix regressions
Browse files Browse the repository at this point in the history 
By using layout compatibility in "gtNewStructVal" instead of handle equality.

This is safe to do because "gtNewStructVal" is only used in contexts of block
copies (as opposed to call arguments).
  • Loading branch information
@SingleAccretion
SingleAccretion committed May 14, 2022
1 parent 37a967a commit 565ef7f
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 28 deletions.
3 changes: 2 additions & 1 deletion src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2303,9 +2303,10 @@ class Compiler
void gtBlockOpInit(GenTree* result, GenTree* dst, GenTree* srcOrFillVal, bool isVolatile);

public:
GenTreeObj* gtNewObjNode(ClassLayout* layout, GenTree* addr);
GenTreeObj* gtNewObjNode(CORINFO_CLASS_HANDLE structHnd, GenTree* addr);
void gtSetObjGcInfo(GenTreeObj* objNode);
GenTree* gtNewStructVal(CORINFO_CLASS_HANDLE structHnd, GenTree* addr);
GenTree* gtNewStructVal(ClassLayout* layout, GenTree* addr);
GenTree* gtNewBlockVal(GenTree* addr, unsigned size);

GenTree* gtNewCpObjNode(GenTree* dst, GenTree* src, CORINFO_CLASS_HANDLE structHnd, bool isVolatile);
Expand Down
68 changes: 45 additions & 23 deletions src/coreclr/jit/gentree.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7196,21 +7196,20 @@ GenTreeOp* Compiler::gtNewAssignNode(GenTree* dst, GenTree* src)
}

//------------------------------------------------------------------------
// gtNewObjNode: Creates a new Obj node.
// gtNewObjNode: Creates a new Obj node with the given layout.
//
// Arguments:
// structHnd - The class handle of the struct type.
// addr - The address of the struct.
// layout - The struct layout
// addr - The address of the struct
//
// Return Value:
// Returns a node representing the struct value at the given address.
//
GenTreeObj* Compiler::gtNewObjNode(CORINFO_CLASS_HANDLE structHnd, GenTree* addr)
GenTreeObj* Compiler::gtNewObjNode(ClassLayout* layout, GenTree* addr)
{
var_types nodeType = impNormStructType(structHnd);
assert(varTypeIsStruct(nodeType));
assert(layout != nullptr);

GenTreeObj* objNode = new (this, GT_OBJ) GenTreeObj(nodeType, addr, typGetObjLayout(structHnd));
GenTreeObj* objNode = new (this, GT_OBJ) GenTreeObj(layout->GetType(), addr, layout);

// An Obj is not a global reference, if it is known to be a local struct.
if ((addr->gtFlags & GTF_GLOB_REF) == 0)
Expand All @@ -7225,6 +7224,25 @@ GenTreeObj* Compiler::gtNewObjNode(CORINFO_CLASS_HANDLE structHnd, GenTree* addr
}
}
}

return objNode;
}

//------------------------------------------------------------------------
// gtNewObjNode: Creates a new Obj node with the layout for the given handle.
//
// Arguments:
// structHnd - The class handle of the struct type
// addr - The address of the struct
//
// Return Value:
// Returns a node representing the struct value at the given address.
//
GenTreeObj* Compiler::gtNewObjNode(CORINFO_CLASS_HANDLE structHnd, GenTree* addr)
{
ClassLayout* layout = typGetObjLayout(structHnd);
GenTreeObj* objNode = gtNewObjNode(layout, addr);

return objNode;
}

Expand All @@ -7233,7 +7251,7 @@ GenTreeObj* Compiler::gtNewObjNode(CORINFO_CLASS_HANDLE structHnd, GenTree* addr
//
// Arguments:
// objNode - The object node of interest

//
void Compiler::gtSetObjGcInfo(GenTreeObj* objNode)
{
assert(varTypeIsStruct(objNode->TypeGet()));
Expand All @@ -7249,28 +7267,31 @@ void Compiler::gtSetObjGcInfo(GenTreeObj* objNode)
// gtNewStructVal: Return a node that represents a struct value
//
// Arguments:
// structHnd - The class for the struct
// addr - The address of the struct
// layout - The struct's layout
// addr - The address of the struct
//
// Return Value:
// A block, object or local node that represents the struct value pointed to by 'addr'.

GenTree* Compiler::gtNewStructVal(CORINFO_CLASS_HANDLE structHnd, GenTree* addr)
// An "OBJ" node, or "LCL_VAR" node if "addr" points to a local with
// a layout compatible with "layout".
//
GenTree* Compiler::gtNewStructVal(ClassLayout* layout, GenTree* addr)
{
if (addr->gtOper == GT_ADDR)
if (addr->OperIs(GT_ADDR))
{
GenTree* val = addr->gtGetOp1();
if (val->OperGet() == GT_LCL_VAR)
GenTree* location = addr->gtGetOp1();
if (location->OperIs(GT_LCL_VAR))
{
unsigned lclNum = addr->gtGetOp1()->AsLclVarCommon()->GetLclNum();
LclVarDsc* varDsc = &(lvaTable[lclNum]);
if (varTypeIsStruct(varDsc) && (varDsc->GetStructHnd() == structHnd) && !lvaIsImplicitByRefLocal(lclNum))
unsigned lclNum = location->AsLclVar()->GetLclNum();
LclVarDsc* varDsc = lvaGetDesc(lclNum);
if (!lvaIsImplicitByRefLocal(lclNum) && varTypeIsStruct(varDsc) &&
ClassLayout::AreCompatible(layout, varDsc->GetLayout()))
{
return addr->gtGetOp1();
return location;
}
}
}
return gtNewObjNode(structHnd, addr);

return gtNewObjNode(layout, addr);
}

//------------------------------------------------------------------------
Expand Down Expand Up @@ -7320,8 +7341,9 @@ GenTree* Compiler::gtNewBlockVal(GenTree* addr, unsigned size)

GenTree* Compiler::gtNewCpObjNode(GenTree* dstAddr, GenTree* srcAddr, CORINFO_CLASS_HANDLE structHnd, bool isVolatile)
{
GenTree* lhs = gtNewStructVal(structHnd, dstAddr);
GenTree* src = gtNewStructVal(structHnd, srcAddr);
ClassLayout* layout = typGetObjLayout(structHnd);
GenTree* lhs = gtNewStructVal(layout, dstAddr);
GenTree* src = gtNewStructVal(layout, srcAddr);

if (lhs->OperIs(GT_OBJ))
{
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/importer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17203,7 +17203,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)

if (eeIsValueClass(resolvedToken.hClass))
{
op1 = gtNewStructVal(resolvedToken.hClass, op1);
op1 = gtNewStructVal(typGetObjLayout(resolvedToken.hClass), op1);
if (op1->OperIs(GT_OBJ))
{
gtSetObjGcInfo(op1->AsObj());
Expand Down
16 changes: 14 additions & 2 deletions src/coreclr/jit/layout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,12 +334,15 @@ ClassLayout* ClassLayout::Create(Compiler* compiler, CORINFO_CLASS_HANDLE classH
size = compiler->info.compCompHnd->getHeapClassSize(classHandle);
}

var_types type = compiler->impNormStructType(classHandle);

INDEBUG(const char* className = compiler->eeGetClassName(classHandle);)
INDEBUG(const char16_t* shortClassName = compiler->eeGetShortClassName(classHandle);)

ClassLayout* layout = new (compiler, CMK_ClassLayout)
ClassLayout(classHandle, isValueClass, size DEBUGARG(className) DEBUGARG(shortClassName));
ClassLayout(classHandle, isValueClass, size, type DEBUGARG(className) DEBUGARG(shortClassName));
layout->InitializeGCPtrs(compiler);

return layout;
}

Expand Down Expand Up @@ -400,7 +403,11 @@ void ClassLayout::InitializeGCPtrs(Compiler* compiler)
// static
bool ClassLayout::AreCompatible(const ClassLayout* layout1, const ClassLayout* layout2)
{
assert((layout1 != nullptr) && (layout2 != nullptr));
if ((layout1 == nullptr) || (layout2 == nullptr))
{
return false;
}

CORINFO_CLASS_HANDLE clsHnd1 = layout1->GetClassHandle();
CORINFO_CLASS_HANDLE clsHnd2 = layout2->GetClassHandle();

Expand All @@ -419,6 +426,11 @@ bool ClassLayout::AreCompatible(const ClassLayout* layout1, const ClassLayout* l
return false;
}

if (layout1->GetType() != layout2->GetType())
{
return false;
}

if (!layout1->HasGCPtr() && !layout2->HasGCPtr())
{
return true;
Expand Down
13 changes: 12 additions & 1 deletion src/coreclr/jit/layout.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ class ClassLayout
BYTE m_gcPtrsArray[sizeof(BYTE*)];
};

// The normalized type to use in IR for block nodes with this layout.
const var_types m_type;

// Class name as reported by ICorJitInfo::getClassName
INDEBUG(const char* m_className;)

Expand All @@ -53,6 +56,7 @@ class ClassLayout
#endif
, m_gcPtrCount(0)
, m_gcPtrs(nullptr)
, m_type(TYP_STRUCT)
#ifdef DEBUG
, m_className("block")
, m_shortClassName(u"block")
Expand All @@ -64,7 +68,8 @@ class ClassLayout

ClassLayout(CORINFO_CLASS_HANDLE classHandle,
bool isValueClass,
unsigned size DEBUGARG(const char* className) DEBUGARG(const char16_t* shortClassName))
unsigned size,
var_types type DEBUGARG(const char* className) DEBUGARG(const char16_t* shortClassName))
: m_classHandle(classHandle)
, m_size(size)
, m_isValueClass(isValueClass)
Expand All @@ -73,6 +78,7 @@ class ClassLayout
#endif
, m_gcPtrCount(0)
, m_gcPtrs(nullptr)
, m_type(type)
#ifdef DEBUG
, m_className(className)
, m_shortClassName(shortClassName)
Expand Down Expand Up @@ -120,6 +126,11 @@ class ClassLayout
return m_size;
}

var_types GetType() const
{
return m_type;
}

//------------------------------------------------------------------------
// GetRegisterType: Determine register type for the layout.
//
Expand Down

0 comments on commit 565ef7f

Please sign in to comment.