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

Jump to bottom

Fix ldelem[a]/stelem/cpobj/ldobj/stobj <pointer type> import #71679

Merged
merged 14 commits into from
Jul 28, 2022
Merged

Fix ldelem[a]/stelem/cpobj/ldobj/stobj <pointer type> import #71679

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
8c66661
Add a test for the ldelema case
SingleAccretion Jul 5, 2022
473cfce
Fix the ldelema case
SingleAccretion Jul 5, 2022
f88818c
Add a test for the ldelem case
SingleAccretion Jul 5, 2022
c806655
Fix the ldelem case
SingleAccretion Jul 5, 2022
088e81f
Add a test for the stelem case
SingleAccretion Jul 6, 2022
5ea93e6
Fix the stelem case
SingleAccretion Jul 6, 2022
b5d27df
Delete dead code while we're here
SingleAccretion Jul 6, 2022
607798f
Add a test for the cpobj case
SingleAccretion Jul 6, 2022
125dd5c
Fix the cpobj case
SingleAccretion Jul 6, 2022
08bcd9f
Fix stobj for consistency
SingleAccretion Jul 6, 2022
b4d8950
Add a test for the ldobj case
SingleAccretion Jul 6, 2022
689ac11
Fix the ldobj case
SingleAccretion Jul 6, 2022
9b76351
Tighten asserts
SingleAccretion Jul 6, 2022
6062b23
Add a TODO-CQ
SingleAccretion Jul 6, 2022
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
182 changes: 52 additions & 130 deletions src/coreclr/jit/importer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4197,12 +4197,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,
switch (ni)
{
case NI_System_Type_get_IsValueType:
retNode = gtNewIconNode(
(eeIsValueClass(hClass) &&
// pointers are not value types (e.g. typeof(int*).IsValueType is false)
info.compCompHnd->asCorInfoType(hClass) != CORINFO_TYPE_PTR)
? 1
: 0);
retNode = gtNewIconNode(eeIsValueClass(hClass) ? 1 : 0);
break;
case NI_System_Type_get_IsByRefLike:
retNode = gtNewIconNode(
Expand Down Expand Up @@ -13669,70 +13664,57 @@ void Compiler::impImportBlockCode(BasicBlock* block)
goto APPEND;

case CEE_LDELEMA:
{
assertImp(sz == sizeof(unsigned));

_impResolveToken(CORINFO_TOKENKIND_Class);

JITDUMP(" %08X", resolvedToken.token);

ldelemClsHnd = resolvedToken.hClass;
lclTyp = JITtype2varType(info.compCompHnd->asCorInfoType(ldelemClsHnd));

// If it's a value class array we just do a simple address-of
if (eeIsValueClass(ldelemClsHnd))
{
CorInfoType cit = info.compCompHnd->getTypeForPrimitiveValueClass(ldelemClsHnd);
if (cit == CORINFO_TYPE_UNDEF)
{
lclTyp = TYP_STRUCT;
}
else
{
lclTyp = JITtype2varType(cit);
}
goto ARR_LD;
}

// Similarly, if its a readonly access, we can do a simple address-of
// without doing a runtime type-check
if (prefixFlags & PREFIX_READONLY)
// If it's a value class / pointer array, or a readonly access, we don't need a type check.
// TODO-CQ: adapt "impCanSkipCovariantStoreCheck" to handle "ldelema"s and call it here to
// skip using the helper in more cases.
if ((lclTyp != TYP_REF) || ((prefixFlags & PREFIX_READONLY) != 0))
SingleAccretion marked this conversation as resolved.
Show resolved Hide resolved
{
lclTyp = TYP_REF;
goto ARR_LD;
}

// Otherwise we need the full helper function with run-time type check
op1 = impTokenToHandle(&resolvedToken);
if (op1 == nullptr)
{ // compDonotInline()
GenTree* type = impTokenToHandle(&resolvedToken);
if (type == nullptr)
{
assert(compDonotInline());
return;
}

{
GenTree* type = op1;
GenTree* index = impPopStack().val;
GenTree* arr = impPopStack().val;
GenTree* index = impPopStack().val;
GenTree* arr = impPopStack().val;

#ifdef TARGET_64BIT
// The CLI Spec allows an array to be indexed by either an int32 or a native int.
// The array helper takes a native int for array length.
// So if we have an int, explicitly extend it to be a native int.
if (genActualType(index->TypeGet()) != TYP_I_IMPL)
// The CLI Spec allows an array to be indexed by either an int32 or a native int.
// The array helper takes a native int for array length.
// So if we have an int, explicitly extend it to be a native int.
if (genActualType(index->TypeGet()) != TYP_I_IMPL)
{
if (index->IsIntegralConst())
{
if (index->IsIntegralConst())
{
index->gtType = TYP_I_IMPL;
}
else
{
bool isUnsigned = false;
index = gtNewCastNode(TYP_I_IMPL, index, isUnsigned, TYP_I_IMPL);
}
index->gtType = TYP_I_IMPL;
}
else
{
bool isUnsigned = false;
index = gtNewCastNode(TYP_I_IMPL, index, isUnsigned, TYP_I_IMPL);
}
#endif // TARGET_64BIT
op1 = gtNewHelperCallNode(CORINFO_HELP_LDELEMA_REF, TYP_BYREF, arr, index, type);
}
#endif // TARGET_64BIT

op1 = gtNewHelperCallNode(CORINFO_HELP_LDELEMA_REF, TYP_BYREF, arr, index, type);
impPushOnStack(op1, tiRetVal);
break;
}
break;

// ldelem for reference and value types
case CEE_LDELEM:
SingleAccretion marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -13743,21 +13725,9 @@ void Compiler::impImportBlockCode(BasicBlock* block)
JITDUMP(" %08X", resolvedToken.token);

ldelemClsHnd = resolvedToken.hClass;

// If it's a reference type or generic variable type
// then just generate code as though it's a ldelem.ref instruction
if (!eeIsValueClass(ldelemClsHnd))
{
lclTyp = TYP_REF;
opcode = CEE_LDELEM_REF;
}
else
{
CorInfoType jitTyp = info.compCompHnd->asCorInfoType(ldelemClsHnd);
lclTyp = JITtype2varType(jitTyp);
tiRetVal = verMakeTypeInfo(ldelemClsHnd); // precise type always needed for struct
tiRetVal.NormaliseForStack();
}
lclTyp = JITtype2varType(info.compCompHnd->asCorInfoType(ldelemClsHnd));
tiRetVal = verMakeTypeInfo(ldelemClsHnd);
tiRetVal.NormaliseForStack();
goto ARR_LD;

case CEE_LDELEM_I1:
Expand Down Expand Up @@ -13835,23 +13805,15 @@ void Compiler::impImportBlockCode(BasicBlock* block)
JITDUMP(" %08X", resolvedToken.token);

stelemClsHnd = resolvedToken.hClass;
lclTyp = JITtype2varType(info.compCompHnd->asCorInfoType(stelemClsHnd));

// If it's a reference type just behave as though it's a stelem.ref instruction
if (!eeIsValueClass(stelemClsHnd))
if (lclTyp != TYP_REF)
{
goto STELEM_REF_POST_VERIFY;
}

// Otherwise extract the type
{
CorInfoType jitTyp = info.compCompHnd->asCorInfoType(stelemClsHnd);
lclTyp = JITtype2varType(jitTyp);
goto ARR_ST;
}
FALLTHROUGH;

case CEE_STELEM_REF:
STELEM_REF_POST_VERIFY:

{
GenTree* value = impStackTop(0).val;
GenTree* index = impStackTop(1).val;
Expand Down Expand Up @@ -17350,20 +17312,16 @@ void Compiler::impImportBlockCode(BasicBlock* block)

JITDUMP(" %08X", resolvedToken.token);

if (!eeIsValueClass(resolvedToken.hClass))
lclTyp = JITtype2varType(info.compCompHnd->asCorInfoType(resolvedToken.hClass));

if (lclTyp != TYP_STRUCT)
{
op1 = impPopStack().val; // address to load from

impBashVarAddrsToI(op1);

assertImp(genActualType(op1->gtType) == TYP_I_IMPL || op1->gtType == TYP_BYREF);

op1 = gtNewOperNode(GT_IND, TYP_REF, op1);
op1->gtFlags |= GTF_EXCEPT | GTF_GLOB_REF;
op1 = gtNewIndir(lclTyp, op1);
op1->gtFlags |= GTF_GLOB_REF;

impPushOnStack(op1, typeInfo());
opcode = CEE_STIND_REF;
lclTyp = TYP_REF;
goto STIND;
}

Expand All @@ -17380,25 +17338,10 @@ void Compiler::impImportBlockCode(BasicBlock* block)

JITDUMP(" %08X", resolvedToken.token);

if (eeIsValueClass(resolvedToken.hClass))
{
lclTyp = TYP_STRUCT;
}
else
{
lclTyp = TYP_REF;
}

if (lclTyp == TYP_REF)
{
opcode = CEE_STIND_REF;
goto STIND;
}
lclTyp = JITtype2varType(info.compCompHnd->asCorInfoType(resolvedToken.hClass));

CorInfoType jitTyp = info.compCompHnd->asCorInfoType(resolvedToken.hClass);
if (impIsPrimitive(jitTyp))
if (lclTyp != TYP_STRUCT)
{
lclTyp = JITtype2varType(jitTyp);
goto STIND;
}

Expand Down Expand Up @@ -17465,39 +17408,19 @@ void Compiler::impImportBlockCode(BasicBlock* block)
JITDUMP(" %08X", resolvedToken.token);

OBJ:

lclTyp = JITtype2varType(info.compCompHnd->asCorInfoType(resolvedToken.hClass));
tiRetVal = verMakeTypeInfo(resolvedToken.hClass);

if (eeIsValueClass(resolvedToken.hClass))
if (lclTyp != TYP_STRUCT)
{
lclTyp = TYP_STRUCT;
}
else
{
lclTyp = TYP_REF;
opcode = CEE_LDIND_REF;
goto LDIND;
}

op1 = impPopStack().val;

assertImp(op1->TypeGet() == TYP_BYREF || op1->TypeGet() == TYP_I_IMPL);

CorInfoType jitTyp = info.compCompHnd->asCorInfoType(resolvedToken.hClass);
if (impIsPrimitive(jitTyp))
{
op1 = gtNewOperNode(GT_IND, JITtype2varType(jitTyp), op1);
assertImp((genActualType(op1) == TYP_I_IMPL) || op1->TypeIs(TYP_BYREF));

// Could point anywhere, example a boxed class static int
op1->gtFlags |= GTF_GLOB_REF;
assertImp(varTypeIsArithmetic(op1->gtType));
}
else
{
// OBJ returns a struct
// and an inline argument which is the class token of the loaded obj
op1 = gtNewObjNode(resolvedToken.hClass, op1);
}
op1 = gtNewObjNode(resolvedToken.hClass, op1);
op1->gtFlags |= GTF_EXCEPT;

if (prefixFlags & PREFIX_UNALIGNED)
Expand Down Expand Up @@ -17810,10 +17733,10 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)
impBashVarAddrsToI(op2);
op2 = impImplicitIorI4Cast(op2, info.compRetType);
op2 = impImplicitR4orR8Cast(op2, info.compRetType);
// Note that we allow TYP_I_IMPL<->TYP_BYREF transformation, but only TYP_I_IMPL<-TYP_REF.

assertImp((genActualType(op2->TypeGet()) == genActualType(info.compRetType)) ||
((op2->TypeGet() == TYP_I_IMPL) && TypeIs(info.compRetType, TYP_BYREF)) ||
(op2->TypeIs(TYP_BYREF, TYP_REF) && (info.compRetType == TYP_I_IMPL)) ||
((op2->TypeGet() == TYP_I_IMPL) && (info.compRetType == TYP_BYREF)) ||
((op2->TypeGet() == TYP_BYREF) && (info.compRetType == TYP_I_IMPL)) ||
(varTypeIsFloating(op2->gtType) && varTypeIsFloating(info.compRetType)) ||
(varTypeIsStruct(op2) && varTypeIsStruct(info.compRetType)));

Expand Down Expand Up @@ -17867,9 +17790,8 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)
if (returnType != originalCallType)
{
// Allow TYP_BYREF to be returned as TYP_I_IMPL and vice versa.
// Allow TYP_REF to be returned as TYP_I_IMPL and NOT vice verse.
if ((TypeIs(returnType, TYP_BYREF, TYP_REF) && (originalCallType == TYP_I_IMPL)) ||
((returnType == TYP_I_IMPL) && TypeIs(originalCallType, TYP_BYREF)))
if (((returnType == TYP_BYREF) && (originalCallType == TYP_I_IMPL)) ||
((returnType == TYP_I_IMPL) && (originalCallType == TYP_BYREF)))
{
JITDUMP("Allowing return type mismatch: have %s, needed %s\n", varTypeName(returnType),
varTypeName(originalCallType));
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/layout.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ ClassLayout* Compiler::typGetObjLayout(CORINFO_CLASS_HANDLE classHandle)

ClassLayout* ClassLayout::Create(Compiler* compiler, CORINFO_CLASS_HANDLE classHandle)
{
bool isValueClass = compiler->info.compCompHnd->isValueClass(classHandle);
bool isValueClass = compiler->eeIsValueClass(classHandle);
unsigned size;

if (isValueClass)
Expand Down
Loading