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.

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

Disallow IND<struct> except as a source of STORE_DYN_BLK #74784

Merged
merged 10 commits into from
Dec 15, 2022
2 changes: 2 additions & 0 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4692,6 +4692,8 @@ bool Compiler::optAssertionIsNonNull(GenTree* op,
return true;
}

op = op->gtEffectiveVal(/* commaOnly */ true);

if (!op->OperIs(GT_LCL_VAR))
{
return false;
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/forwardsub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -632,10 +632,10 @@ bool Compiler::fgForwardSubStatement(Statement* stmt)

// Quirks:
//
// Don't substitute nodes "AddFinalArgsAndDetermineABIInfo" doesn't handle into struct args.
// Don't substitute nodes args morphing doesn't handle into struct args.
//
if (fsv.IsCallArg() && fsv.GetNode()->TypeIs(TYP_STRUCT) &&
!fwdSubNode->OperIs(GT_OBJ, GT_LCL_VAR, GT_LCL_FLD, GT_MKREFANY))
!fwdSubNode->OperIs(GT_OBJ, GT_FIELD, GT_LCL_VAR, GT_LCL_FLD, GT_MKREFANY))
{
JITDUMP(" use is a struct arg; fwd sub node is not OBJ/LCL_VAR/LCL_FLD/MKREFANY\n");
return false;
Expand Down
7 changes: 3 additions & 4 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15718,12 +15718,11 @@ GenTree* Compiler::gtNewTempAssign(
if (varTypeIsStruct(varDsc) && (valStructHnd == NO_CLASS_HANDLE) && !varTypeIsSIMD(valTyp))
{
// There are some cases where we do not have a struct handle on the return value:
// 1. Handle-less IND/BLK/LCL_FLD<struct> nodes.
// 1. Handle-less BLK/LCL_FLD nodes.
// 2. The zero constant created by local assertion propagation.
// In these cases, we can use the type of the merge return for the assignment.
// In these cases, we can use the type of the local for the assignment.
assert(val->gtEffectiveVal(true)->OperIs(GT_IND, GT_BLK, GT_LCL_FLD, GT_CNS_INT));
assert(tmp == genReturnLocal);
valStructHnd = lvaGetStruct(genReturnLocal);
valStructHnd = lvaGetDesc(tmp)->GetStructHnd();
assert(valStructHnd != NO_CLASS_HANDLE);
}

Expand Down
28 changes: 12 additions & 16 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -7157,29 +7157,25 @@ struct GenTreeBlk : public GenTreeIndir

GenTreeBlk(genTreeOps oper, var_types type, GenTree* addr, ClassLayout* layout)
: GenTreeIndir(oper, type, addr, nullptr)
, m_layout(layout)
, gtBlkOpKind(BlkOpKindInvalid)
#ifndef JIT32_GCENCODER
, gtBlkOpGcUnsafe(false)
#endif
{
assert(OperIsBlk(oper));
assert((layout != nullptr) || OperIs(GT_STORE_DYN_BLK));
gtFlags |= (addr->gtFlags & GTF_ALL_EFFECT);
Initialize(layout);
}

GenTreeBlk(genTreeOps oper, var_types type, GenTree* addr, GenTree* data, ClassLayout* layout)
: GenTreeIndir(oper, type, addr, data)
, m_layout(layout)
, gtBlkOpKind(BlkOpKindInvalid)
{
Initialize(layout);
}

void Initialize(ClassLayout* layout)
{
assert(OperIsBlk(OperGet()) && ((layout != nullptr) || OperIs(GT_STORE_DYN_BLK)));

m_layout = layout;
gtBlkOpKind = BlkOpKindInvalid;
#ifndef JIT32_GCENCODER
, gtBlkOpGcUnsafe(false)
gtBlkOpGcUnsafe = false;
#endif
{
assert(OperIsBlk(oper));
assert((layout != nullptr) || OperIs(GT_STORE_DYN_BLK));
gtFlags |= (addr->gtFlags & GTF_ALL_EFFECT);
gtFlags |= (data->gtFlags & GTF_ALL_EFFECT);
}

#if DEBUGGABLE_GENTREE
Expand Down
137 changes: 41 additions & 96 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -934,18 +934,11 @@ GenTree* Compiler::impAssignStruct(GenTree* dest,
assert(varTypeIsStruct(dest) && (dest->OperIsLocal() || dest->OperIsIndir() || dest->OperIs(GT_FIELD)));

assert(dest->TypeGet() == src->TypeGet());
// TODO-1stClassStructs: delete the "!IND" condition once "IND<struct>" nodes are no more.
if (dest->TypeIs(TYP_STRUCT) && !src->gtEffectiveVal()->OperIs(GT_IND))
if (dest->TypeIs(TYP_STRUCT))
{
assert(ClassLayout::AreCompatible(dest->GetLayout(this), src->GetLayout(this)));
}

if (dest->OperIs(GT_FIELD) && dest->TypeIs(TYP_STRUCT))
{
// TODO-ADDR: delete this once FIELD<struct> nodes are transformed into OBJs (not INDs).
dest = gtNewObjNode(dest->GetLayout(this), gtNewOperNode(GT_ADDR, TYP_BYREF, dest));
}

DebugInfo usedDI = di;
if (!usedDI.IsValid())
{
Expand Down Expand Up @@ -1338,67 +1331,37 @@ GenTree* Compiler::impNormStructVal(GenTree* structVal, CORINFO_CLASS_HANDLE str
{
structType = impNormStructType(structHnd);
}
bool alreadyNormalized = false;
GenTreeLclVarCommon* structLcl = nullptr;
GenTreeLclVarCommon* structLcl = nullptr;

genTreeOps oper = structVal->OperGet();
switch (oper)
switch (structVal->OperGet())
{
// GT_MKREFANY is supported directly by args morphing.
case GT_MKREFANY:
alreadyNormalized = true;
break;

case GT_CALL:
case GT_RET_EXPR:
makeTemp = true;
break;

case GT_FIELD:
// Wrap it in a GT_OBJ, if needed.
structVal->gtType = structType;
if (structType == TYP_STRUCT)
{
structVal = gtNewObjNode(structHnd, gtNewOperNode(GT_ADDR, TYP_BYREF, structVal));
}
break;

case GT_LCL_VAR:
case GT_LCL_FLD:
structLcl = structVal->AsLclVarCommon();
// Wrap it in a GT_OBJ.
structVal = gtNewObjNode(structHnd, gtNewOperNode(GT_ADDR, TYP_BYREF, structVal));
FALLTHROUGH;

case GT_IND:
case GT_OBJ:
case GT_BLK:
// These should already have the appropriate type.
assert(structVal->gtType == structType);
alreadyNormalized = true;
break;

case GT_IND:
assert(structVal->gtType == structType);
structVal = gtNewObjNode(structHnd, structVal->gtGetOp1());
alreadyNormalized = true;
break;

case GT_FIELD:
case GT_CNS_VEC:
assert(varTypeIsSIMD(structVal) && (structVal->gtType == structType));
break;

#ifdef FEATURE_SIMD
case GT_SIMD:
assert(varTypeIsSIMD(structVal) && (structVal->gtType == structType));
break;
#endif // FEATURE_SIMD
#endif
#ifdef FEATURE_HW_INTRINSICS
case GT_HWINTRINSIC:
assert(structVal->gtType == structType);
assert(varTypeIsSIMD(structVal) ||
HWIntrinsicInfo::IsMultiReg(structVal->AsHWIntrinsic()->GetHWIntrinsicId()));
break;
#endif
case GT_MKREFANY:
// These should already have the appropriate type.
assert(structVal->TypeGet() == structType);
break;

case GT_COMMA:
{
Expand All @@ -1419,22 +1382,15 @@ GenTree* Compiler::impNormStructVal(GenTree* structVal, CORINFO_CLASS_HANDLE str
} while (blockNode->OperGet() == GT_COMMA);
}

if (blockNode->OperGet() == GT_FIELD)
{
// If we have a GT_FIELD then wrap it in a GT_OBJ.
blockNode = gtNewObjNode(structHnd, gtNewOperNode(GT_ADDR, TYP_BYREF, blockNode));
}

#ifdef FEATURE_SIMD
if (blockNode->OperIsSimdOrHWintrinsic() || blockNode->IsCnsVec())
{
parent->AsOp()->gtOp2 = impNormStructVal(blockNode, structHnd, curLevel);
alreadyNormalized = true;
}
else
#endif
{
noway_assert(blockNode->OperIsBlk());
noway_assert(blockNode->OperIsBlk() || blockNode->OperIs(GT_FIELD));

// Sink the GT_COMMA below the blockNode addr.
// That is GT_COMMA(op1, op2=blockNode) is transformed into
Expand All @@ -1452,7 +1408,6 @@ GenTree* Compiler::impNormStructVal(GenTree* structVal, CORINFO_CLASS_HANDLE str
{
structVal = blockNode;
}
alreadyNormalized = true;
}
}
break;
Expand All @@ -1461,22 +1416,19 @@ GenTree* Compiler::impNormStructVal(GenTree* structVal, CORINFO_CLASS_HANDLE str
noway_assert(!"Unexpected node in impNormStructVal()");
break;
}
structVal->gtType = structType;

if (!alreadyNormalized)
if (makeTemp)
{
if (makeTemp)
{
unsigned tmpNum = lvaGrabTemp(true DEBUGARG("struct address for call/obj"));
unsigned tmpNum = lvaGrabTemp(true DEBUGARG("struct address for call/obj"));

impAssignTempGen(tmpNum, structVal, structHnd, curLevel);
impAssignTempGen(tmpNum, structVal, structHnd, curLevel);

// The structVal is now the temp itself
// The structVal is now the temp itself

structLcl = gtNewLclvNode(tmpNum, structType)->AsLclVarCommon();
structVal = structLcl;
}
if ((structType == TYP_STRUCT) && !structVal->OperIsBlk())
structLcl = gtNewLclvNode(tmpNum, structType)->AsLclVarCommon();
structVal = structLcl;

if (structType == TYP_STRUCT)
{
// Wrap it in a GT_OBJ
structVal = gtNewObjNode(structHnd, gtNewOperNode(GT_ADDR, TYP_BYREF, structVal));
Expand Down Expand Up @@ -10105,26 +10057,11 @@ void Compiler::impImportBlockCode(BasicBlock* block)
break;
}
case CEE_REFANYTYPE:

{
op1 = impPopStack().val;

// make certain it is normalized;
op1 = impNormStructVal(op1, impGetRefAnyClass(), CHECK_SPILL_ALL);

if (op1->gtOper == GT_OBJ)
if (op1->OperIs(GT_MKREFANY))
{
// Get the address of the refany
op1 = op1->AsOp()->gtOp1;

// Fetch the type from the correct slot
op1 = gtNewOperNode(GT_ADD, TYP_BYREF, op1,
gtNewIconNode(OFFSETOF__CORINFO_TypedReference__type, TYP_I_IMPL));
op1 = gtNewOperNode(GT_IND, TYP_BYREF, op1);
}
else
{
assertImp(op1->gtOper == GT_MKREFANY);

// The pointer may have side-effects
if (op1->AsOp()->gtOp1->gtFlags & GTF_SIDE_EFFECT)
{
Expand All @@ -10137,25 +10074,33 @@ void Compiler::impImportBlockCode(BasicBlock* block)
// We already have the class handle
op1 = op1->AsOp()->gtOp2;
}

// convert native TypeHandle to RuntimeTypeHandle
else
{
op1 = gtNewHelperCallNode(CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPEHANDLE_MAYBENULL, TYP_STRUCT, op1);
// Get the address of the refany
op1 = impGetStructAddr(op1, impGetRefAnyClass(), CHECK_SPILL_ALL, /* willDeref */ true);

// Fetch the type from the correct slot
op1 = gtNewOperNode(GT_ADD, TYP_BYREF, op1,
gtNewIconNode(OFFSETOF__CORINFO_TypedReference__type, TYP_I_IMPL));
op1 = gtNewOperNode(GT_IND, TYP_BYREF, op1);
}

// Convert native TypeHandle to RuntimeTypeHandle.
op1 = gtNewHelperCallNode(CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPEHANDLE_MAYBENULL, TYP_STRUCT, op1);

CORINFO_CLASS_HANDLE classHandle = impGetTypeHandleClass();
CORINFO_CLASS_HANDLE classHandle = impGetTypeHandleClass();

// The handle struct is returned in register
op1->AsCall()->gtReturnType = GetRuntimeHandleUnderlyingType();
op1->AsCall()->gtRetClsHnd = classHandle;
// The handle struct is returned in register
op1->AsCall()->gtReturnType = GetRuntimeHandleUnderlyingType();
op1->AsCall()->gtRetClsHnd = classHandle;
#if FEATURE_MULTIREG_RET
op1->AsCall()->InitializeStructReturnType(this, classHandle, op1->AsCall()->GetUnmanagedCallConv());
op1->AsCall()->InitializeStructReturnType(this, classHandle, op1->AsCall()->GetUnmanagedCallConv());
#endif

tiRetVal = typeInfo(TI_STRUCT, classHandle);
}

tiRetVal = typeInfo(TI_STRUCT, classHandle);
impPushOnStack(op1, tiRetVal);
break;
}
break;

case CEE_LDTOKEN:
{
Expand Down
10 changes: 7 additions & 3 deletions src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2156,9 +2156,13 @@ GenTree* Compiler::impInitializeArrayIntrinsic(CORINFO_SIG_INFO* sig)
dataOffset = eeGetArrayDataOffset();
}

GenTree* dstAddr = gtNewOperNode(GT_ADD, TYP_BYREF, arrayLocalNode, gtNewIconNode(dataOffset, TYP_I_IMPL));
GenTree* dst = new (this, GT_BLK) GenTreeBlk(GT_BLK, TYP_STRUCT, dstAddr, typGetBlkLayout(blkSize));
GenTree* src = gtNewIndOfIconHandleNode(TYP_STRUCT, (size_t)initData, GTF_ICON_CONST_PTR, true);
ClassLayout* blkLayout = typGetBlkLayout(blkSize);
GenTree* dstAddr = gtNewOperNode(GT_ADD, TYP_BYREF, arrayLocalNode, gtNewIconNode(dataOffset, TYP_I_IMPL));
GenTree* dst = gtNewStructVal(blkLayout, dstAddr);
dst->gtFlags |= GTF_GLOB_REF;

GenTree* srcAddr = gtNewIconHandleNode((size_t)initData, GTF_ICON_CONST_PTR);
GenTree* src = gtNewStructVal(blkLayout, srcAddr);

#ifdef DEBUG
src->gtGetOp1()->AsIntCon()->gtTargetHandle = THT_InitializeArrayIntrinsics;
Expand Down
12 changes: 2 additions & 10 deletions src/coreclr/jit/lclmorph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -899,11 +899,7 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
{
ClassLayout* layout = node->GetLayout(m_compiler);
node->SetOper(GT_OBJ);
node->AsBlk()->SetLayout(layout);
node->AsBlk()->gtBlkOpKind = GenTreeBlk::BlkOpKindInvalid;
#ifndef JIT32_GCENCODER
node->AsBlk()->gtBlkOpGcUnsafe = false;
#endif // !JIT32_GCENCODER
node->AsBlk()->Initialize(layout);
}
else
{
Expand Down Expand Up @@ -1236,11 +1232,7 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
if (node->TypeIs(TYP_STRUCT))
{
node->SetOper(GT_OBJ);
node->AsBlk()->SetLayout(layout);
node->AsBlk()->gtBlkOpKind = GenTreeBlk::BlkOpKindInvalid;
#ifndef JIT32_GCENCODER
node->AsBlk()->gtBlkOpGcUnsafe = false;
#endif // !JIT32_GCENCODER
node->AsBlk()->Initialize(layout);
}
else
{
Expand Down
21 changes: 5 additions & 16 deletions src/coreclr/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3677,13 +3677,10 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
lclStore->ChangeOper(GT_STORE_OBJ);
GenTreeBlk* objStore = lclStore->AsObj();
objStore->gtFlags = GTF_ASG | GTF_IND_NONFAULTING | GTF_IND_TGT_NOT_HEAP;
#ifndef JIT32_GCENCODER
objStore->gtBlkOpGcUnsafe = false;
#endif
objStore->gtBlkOpKind = GenTreeObj::BlkOpKindInvalid;
objStore->SetLayout(layout);
objStore->Initialize(layout);
objStore->SetAddr(addr);
objStore->SetData(src);

BlockRange().InsertBefore(objStore, addr);
LowerNode(objStore);
return;
Expand Down Expand Up @@ -3777,20 +3774,12 @@ void Lowering::LowerRetStruct(GenTreeUnOp* ret)
{
// Spill to a local if sizes don't match so we can avoid the "load more than requested"
// problem, e.g. struct size is 5 and we emit "ldr x0, [x1]"
unsigned realSize = retVal->AsIndir()->Size();
CORINFO_CLASS_HANDLE structCls = comp->info.compMethodInfo->args.retTypeClass;
if (realSize == 0)
{
// TODO-ADDR: delete once "IND<struct>" nodes are no more
realSize = comp->info.compCompHnd->getClassSize(structCls);
}

if (genTypeSize(nativeReturnType) > realSize)
if (genTypeSize(nativeReturnType) > retVal->AsIndir()->Size())
{
LIR::Use retValUse(BlockRange(), &ret->gtOp1, ret);
unsigned tmpNum = comp->lvaGrabTemp(true DEBUGARG("mis-sized struct return"));
comp->lvaSetStruct(tmpNum, structCls, false);
comp->genReturnLocal = tmpNum;
comp->lvaSetStruct(tmpNum, comp->info.compMethodInfo->args.retTypeClass, false);

ReplaceWithLclVar(retValUse, tmpNum);
LowerRetSingleRegStructLclVar(ret);
break;
Expand Down
Loading