Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

JIT: devirtualization support for EqualityComparer<T>.Default #14125

Merged
merged 1 commit into from
Sep 27, 2017
Merged

JIT: devirtualization support for EqualityComparer<T>.Default #14125

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
12 changes: 10 additions & 2 deletions src/ToolBox/superpmi/superpmi-shared/icorjitinfoimpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,23 @@ CORINFO_METHOD_HANDLE resolveVirtualMethod(CORINFO_METHOD_HANDLE virtualMethod,
CORINFO_CLASS_HANDLE implementingClass,
CORINFO_CONTEXT_HANDLE ownerType);

// Given T, return the type of the default EqualityComparer<T>.
// Returns null if the type can't be determined exactly.
CORINFO_CLASS_HANDLE getDefaultEqualityComparerClass(CORINFO_CLASS_HANDLE elemType);

// Given resolved token that corresponds to an intrinsic classified as
// a CORINFO_INTRINSIC_GetRawHandle intrinsic, fetch the handle associated
// with the token. If this is not possible at compile-time (because the current method's
// code is shared and the token contains generic parameters) then indicate
// how the handle should be looked up at runtime.
void expandRawHandleIntrinsic(
CORINFO_RESOLVED_TOKEN * pResolvedToken,
CORINFO_GENERICHANDLE_RESULT * pResult);

// If a method's attributes have (getMethodAttribs) CORINFO_FLG_INTRINSIC set,
// getIntrinsicID() returns the intrinsic ID.
// *pMustExpand tells whether or not JIT must expand the intrinsic.
CorInfoIntrinsics getIntrinsicID(CORINFO_METHOD_HANDLE method, bool* pMustExpand = NULL /* OUT */
);
CorInfoIntrinsics getIntrinsicID(CORINFO_METHOD_HANDLE method, bool* pMustExpand = NULL /* OUT */);

// Is the given module the System.Numerics.Vectors module?
// This defaults to false.
Expand Down
1 change: 1 addition & 0 deletions src/ToolBox/superpmi/superpmi-shared/lwmlist.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ LWM(GetClassName, DWORDLONG, DWORD)
LWM(GetClassNumInstanceFields, DWORDLONG, DWORD)
LWM(GetClassSize, DWORDLONG, DWORD)
LWM(GetCookieForPInvokeCalliSig, GetCookieForPInvokeCalliSigValue, DLDL)
LWM(GetDefaultEqualityComparerClass, DWORDLONG, DWORDLONG)
LWM(GetDelegateCtor, Agnostic_GetDelegateCtorIn, Agnostic_GetDelegateCtorOut)
LWM(GetEEInfo, DWORD, Agnostic_CORINFO_EE_INFO)
LWM(GetEHinfo, DLD, Agnostic_CORINFO_EH_CLAUSE)
Expand Down
17 changes: 17 additions & 0 deletions src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3012,6 +3012,23 @@ CORINFO_METHOD_HANDLE MethodContext::repResolveVirtualMethod(CORINFO_METHOD_HAND
return (CORINFO_METHOD_HANDLE)result;
}

void MethodContext::recGetDefaultEqualityComparerClass(CORINFO_CLASS_HANDLE cls, CORINFO_CLASS_HANDLE result)
{
if (GetDefaultEqualityComparerClass == nullptr)
GetDefaultEqualityComparerClass = new LightWeightMap<DWORDLONG, DWORDLONG>();

GetDefaultEqualityComparerClass->Add((DWORDLONG)cls, (DWORDLONG)result);
}
void MethodContext::dmpGetDefaultEqualityComparerClass(DWORDLONG key, DWORDLONG value)
{
printf("GetDefaultEqualityComparerClass key cls-%016llX, value cls-%016llX", key, value);
}
CORINFO_CLASS_HANDLE MethodContext::repGetDefaultEqualityComparerClass(CORINFO_CLASS_HANDLE cls)
{
CORINFO_CLASS_HANDLE result = (CORINFO_CLASS_HANDLE)GetDefaultEqualityComparerClass->Get((DWORDLONG)cls);
return result;
}

void MethodContext::recGetTokenTypeAsHandle(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_CLASS_HANDLE result)
{
if (GetTokenTypeAsHandle == nullptr)
Expand Down
8 changes: 7 additions & 1 deletion src/ToolBox/superpmi/superpmi-shared/methodcontext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,11 @@ class MethodContext
CORINFO_CLASS_HANDLE implClass,
CORINFO_CONTEXT_HANDLE ownerType);

void recGetDefaultEqualityComparerClass(CORINFO_CLASS_HANDLE cls, CORINFO_CLASS_HANDLE result);
void dmpGetDefaultEqualityComparerClass(DWORDLONG key, DWORDLONG value);
CORINFO_CLASS_HANDLE repGetDefaultEqualityComparerClass(CORINFO_CLASS_HANDLE cls);


void recGetTokenTypeAsHandle(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_CLASS_HANDLE result);
void dmpGetTokenTypeAsHandle(const GetTokenTypeAsHandleValue& key, DWORDLONG value);
CORINFO_CLASS_HANDLE repGetTokenTypeAsHandle(CORINFO_RESOLVED_TOKEN* pResolvedToken);
Expand Down Expand Up @@ -1252,7 +1257,7 @@ class MethodContext
};

// ********************* Please keep this up-to-date to ease adding more ***************
// Highest packet number: 161
// Highest packet number: 162
// *************************************************************************************
enum mcPackets
{
Expand Down Expand Up @@ -1319,6 +1324,7 @@ enum mcPackets
Packet_GetIntConfigValue = 151, // Added 2/12/2015
Packet_GetStringConfigValue = 152, // Added 2/12/2015
Packet_GetCookieForPInvokeCalliSig = 48,
Packet_GetDefaultEqualityComparerClass = 162, // Added 9/24/2017
Packet_GetDelegateCtor = 49,
Packet_GetEEInfo = 50,
Packet_GetEHinfo = 51,
Expand Down
10 changes: 10 additions & 0 deletions src/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,16 @@ CORINFO_METHOD_HANDLE interceptor_ICJI::resolveVirtualMethod(CORINFO_METHOD_HAND
return result;
}

// Given T, return the type of the default EqualityComparer<T>.
// Returns null if the type can't be determined exactly.
CORINFO_CLASS_HANDLE interceptor_ICJI::getDefaultEqualityComparerClass(CORINFO_CLASS_HANDLE cls)
{
mc->cr->AddCall("getDefaultEqualityComparerClass");
CORINFO_CLASS_HANDLE result = original_ICorJitInfo->getDefaultEqualityComparerClass(cls);
mc->recGetDefaultEqualityComparerClass(cls, result);
return result;
}

void interceptor_ICJI::expandRawHandleIntrinsic(
CORINFO_RESOLVED_TOKEN * pResolvedToken,
CORINFO_GENERICHANDLE_RESULT * pResult)
Expand Down
8 changes: 8 additions & 0 deletions src/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,14 @@ CORINFO_METHOD_HANDLE interceptor_ICJI::resolveVirtualMethod(CORINFO_METHOD_HAND
return original_ICorJitInfo->resolveVirtualMethod(virtualMethod, implementingClass, ownerType);
}

// Given T, return the type of the default EqualityComparer<T>.
// Returns null if the type can't be determined exactly.
CORINFO_CLASS_HANDLE interceptor_ICJI::getDefaultEqualityComparerClass(CORINFO_CLASS_HANDLE cls)
{
mcs->AddCall("getDefaultEqualityComparerClass");
return original_ICorJitInfo->getDefaultEqualityComparerClass(cls);
}

void interceptor_ICJI::expandRawHandleIntrinsic(
CORINFO_RESOLVED_TOKEN * pResolvedToken,
CORINFO_GENERICHANDLE_RESULT * pResult)
Expand Down
7 changes: 7 additions & 0 deletions src/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,13 @@ CORINFO_METHOD_HANDLE interceptor_ICJI::resolveVirtualMethod(CORINFO_METHOD_HAND
return original_ICorJitInfo->resolveVirtualMethod(virtualMethod, implementingClass, ownerType);
}

// Given T, return the type of the default EqualityComparer<T>.
// Returns null if the type can't be determined exactly.
CORINFO_CLASS_HANDLE interceptor_ICJI::getDefaultEqualityComparerClass(CORINFO_CLASS_HANDLE cls)
{
return original_ICorJitInfo->getDefaultEqualityComparerClass(cls);
}

void interceptor_ICJI::expandRawHandleIntrinsic(
CORINFO_RESOLVED_TOKEN * pResolvedToken,
CORINFO_GENERICHANDLE_RESULT * pResult)
Expand Down
9 changes: 9 additions & 0 deletions src/ToolBox/superpmi/superpmi/icorjitinfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,15 @@ CORINFO_METHOD_HANDLE MyICJI::resolveVirtualMethod(CORINFO_METHOD_HANDLE virtua
return result;
}

// Given T, return the type of the default EqualityComparer<T>.
// Returns null if the type can't be determined exactly.
CORINFO_CLASS_HANDLE MyICJI::getDefaultEqualityComparerClass(CORINFO_CLASS_HANDLE cls)
{
jitInstance->mc->cr->AddCall("getDefaultEqualityComparerClass");
CORINFO_CLASS_HANDLE result = jitInstance->mc->repGetDefaultEqualityComparerClass(cls);
return result;
}

void MyICJI::expandRawHandleIntrinsic(
CORINFO_RESOLVED_TOKEN * pResolvedToken,
CORINFO_GENERICHANDLE_RESULT * pResult)
Expand Down
16 changes: 11 additions & 5 deletions src/inc/corinfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,11 @@ TODO: Talk about initializing strutures before use
#define SELECTANY extern __declspec(selectany)
#endif

SELECTANY const GUID JITEEVersionIdentifier = { /* 7f70c266-eada-427b-be8a-be1260e34b1b */
0x7f70c266,
0xeada,
0x427b,
{0xbe, 0x8a, 0xbe, 0x12, 0x60, 0xe3, 0x4b, 0x1b}
SELECTANY const GUID JITEEVersionIdentifier = { /* 76a743cd-8a07-471e-9ac4-cd5806a8ffac */
0x76a743cd,
0x8a07,
0x471e,
{0x9a, 0xc4, 0xcd, 0x58, 0x06, 0xa8, 0xff, 0xac}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -2085,6 +2085,12 @@ class ICorStaticInfo
CORINFO_CONTEXT_HANDLE ownerType = NULL /* IN */
) = 0;

// Given T, return the type of the default EqualityComparer<T>.
// Returns null if the type can't be determined exactly.
virtual CORINFO_CLASS_HANDLE getDefaultEqualityComparerClass(
CORINFO_CLASS_HANDLE elemType
) = 0;

// Given resolved token that corresponds to an intrinsic classified as
// a CORINFO_INTRINSIC_GetRawHandle intrinsic, fetch the handle associated
// with the token. If this is not possible at compile-time (because the current method's
Expand Down
1 change: 1 addition & 0 deletions src/jit/ICorJitInfo_API_names.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,5 +168,6 @@ DEF_CLR_API(getModuleNativeEntryPointRange)
DEF_CLR_API(getExpectedTargetArchitecture)
DEF_CLR_API(resolveVirtualMethod)
DEF_CLR_API(expandRawHandleIntrinsic)
DEF_CLR_API(getDefaultEqualityComparerClass)

#undef DEF_CLR_API
10 changes: 10 additions & 0 deletions src/jit/ICorJitInfo_API_wrapper.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1620,6 +1620,16 @@ CORINFO_METHOD_HANDLE WrapICorJitInfo::resolveVirtualMethod(
return result;
}

CORINFO_CLASS_HANDLE WrapICorJitInfo::getDefaultEqualityComparerClass(
CORINFO_CLASS_HANDLE elemType)
{
API_ENTER(getDefaultEqualityComparerClass);
CORINFO_CLASS_HANDLE result = wrapHnd->getDefaultEqualityComparerClass(elemType);
API_LEAVE(getDefaultEqualityComparerClass);
return result;
}


void WrapICorJitInfo::expandRawHandleIntrinsic(
CORINFO_RESOLVED_TOKEN * pResolvedToken,
CORINFO_GENERICHANDLE_RESULT * pResult)
Expand Down
3 changes: 2 additions & 1 deletion src/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2966,12 +2966,13 @@ class Compiler
IL_OFFSET rawILOffset);

void impDevirtualizeCall(GenTreeCall* call,
GenTreePtr thisObj,
CORINFO_METHOD_HANDLE* method,
unsigned* methodFlags,
CORINFO_CONTEXT_HANDLE* contextHandle,
CORINFO_CONTEXT_HANDLE* exactContextHandle);

CORINFO_CLASS_HANDLE impGetSpecialIntrinsicExactReturnType(CORINFO_METHOD_HANDLE specialIntrinsicHandle);

bool impMethodInfo_hasRetBuffArg(CORINFO_METHOD_INFO* methInfo);

GenTreePtr impFixupCallStructReturn(GenTreeCall* call, CORINFO_CLASS_HANDLE retClsHnd);
Expand Down
107 changes: 99 additions & 8 deletions src/jit/flowgraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7056,6 +7056,19 @@ GenTreeCall* Compiler::fgGetStaticsCCtorHelper(CORINFO_CLASS_HANDLE cls, CorInfo
GenTreeCall* result = gtNewHelperCallNode(helper, type, argList);
result->gtFlags |= callFlags;

// If we're importing the special EqualityComparer<T>.Default
// intrinsic, flag the helper call. Later during inlining, we can
// remove the helper call if the associated field lookup is unused.
if ((info.compFlags & CORINFO_FLG_JIT_INTRINSIC) != 0)
{
NamedIntrinsic ni = lookupNamedIntrinsic(info.compMethodHnd);
if (ni == NI_System_Collections_Generic_EqualityComparer_get_Default)
{
JITDUMP("\nmarking helper call [06%u] as special dce...\n", result->gtTreeID);
result->gtCallMoreFlags |= GTF_CALL_M_HELPER_SPECIAL_DCE;
}
}

return result;
}

Expand Down Expand Up @@ -22233,7 +22246,7 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr
CORINFO_METHOD_HANDLE method = call->gtCallMethHnd;
unsigned methodFlags = 0;
CORINFO_CONTEXT_HANDLE context = nullptr;
comp->impDevirtualizeCall(call, tree, &method, &methodFlags, &context, nullptr);
comp->impDevirtualizeCall(call, &method, &methodFlags, &context, nullptr);
}
}
}
Expand Down Expand Up @@ -22928,8 +22941,8 @@ GenTreePtr Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
GenTreeStmt* callStmt = inlineInfo->iciStmt;
IL_OFFSETX callILOffset = callStmt->gtStmtILoffsx;
GenTreeStmt* postStmt = callStmt->gtNextStmt;
GenTreePtr afterStmt = callStmt; // afterStmt is the place where the new statements should be inserted after.
GenTreePtr newStmt = nullptr;
GenTree* afterStmt = callStmt; // afterStmt is the place where the new statements should be inserted after.
GenTree* newStmt = nullptr;
GenTreeCall* call = inlineInfo->iciCall->AsCall();

noway_assert(call->gtOper == GT_CALL);
Expand Down Expand Up @@ -23074,6 +23087,8 @@ GenTreePtr Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
if (argInfo.argHasSideEff)
{
noway_assert(argInfo.argIsUsed == false);
newStmt = nullptr;
bool append = true;

if (argNode->gtOper == GT_OBJ || argNode->gtOper == GT_MKREFANY)
{
Expand All @@ -23084,16 +23099,92 @@ GenTreePtr Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
}
else
{
newStmt = gtNewStmt(gtUnusedValNode(argNode), callILOffset);
// In some special cases, unused args with side effects can
// trigger further changes.
//
// (1) If the arg is a static field access and the field access
// was produced by a call to EqualityComparer<T>.get_Default, the
// helper call to ensure the field has a value can be suppressed.
// This helper call is marked as a "Special DCE" helper during
// importation, over in fgGetStaticsCCtorHelper.
//
// (2) NYI. If, after tunneling through GT_RET_VALs, we find that
// the actual arg expression has no side effects, we can skip
// appending all together. This will help jit TP a bit.
//
// Chase through any GT_RET_EXPRs to find the actual argument
// expression.
GenTree* actualArgNode = argNode;
while (actualArgNode->gtOper == GT_RET_EXPR)
{
actualArgNode = actualArgNode->gtRetExpr.gtInlineCandidate;
}

// For case (1)
//
// Look for the following tree shapes
// prejit: (IND (ADD (CONST, CALL(special dce helper...))))
// jit : (COMMA (CALL(special dce helper...), (FIELD ...)))
Copy link

Choose a reason for hiding this comment

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

Unfortunate that these have different patterns, and jit-diff only shows prejit diffs. Any thoughts about how to add regression testing for the jit case?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could possibly catch regressions in a perf test. Or SPMI if we ever get that into CI.

if (actualArgNode->gtOper == GT_COMMA)
{
// Look for (COMMA (CALL(special dce helper...), (FIELD ...)))
GenTree* op1 = actualArgNode->gtOp.gtOp1;
GenTree* op2 = actualArgNode->gtOp.gtOp2;
if (op1->IsCall() && ((op1->gtCall.gtCallMoreFlags & GTF_CALL_M_HELPER_SPECIAL_DCE) != 0) &&
(op2->gtOper == GT_FIELD) && ((op2->gtFlags & GTF_EXCEPT) == 0))
{
JITDUMP("\nPerforming special dce on unused arg [%06u]:"
" actual arg [%06u] helper call [%06u]\n",
argNode->gtTreeID, actualArgNode->gtTreeID, op1->gtTreeID);
// Drop the whole tree
append = false;
}
}
else if (actualArgNode->gtOper == GT_IND)
{
// Look for (IND (ADD (CONST, CALL(special dce helper...))))
GenTree* addr = actualArgNode->gtOp.gtOp1;

if (addr->gtOper == GT_ADD)
{
GenTree* op1 = addr->gtOp.gtOp1;
GenTree* op2 = addr->gtOp.gtOp2;
if (op1->IsCall() &&
((op1->gtCall.gtCallMoreFlags & GTF_CALL_M_HELPER_SPECIAL_DCE) != 0) &&
op2->IsCnsIntOrI())
{
// Drop the whole tree
JITDUMP("\nPerforming special dce on unused arg [%06u]:"
" actual arg [%06u] helper call [%06u]\n",
argNode->gtTreeID, actualArgNode->gtTreeID, op1->gtTreeID);
append = false;
}
}
}
}
afterStmt = fgInsertStmtAfter(block, afterStmt, newStmt);

#ifdef DEBUG
if (verbose)
if (!append)
{
gtDispTree(afterStmt);
assert(newStmt == nullptr);
JITDUMP("Arg tree side effects were discardable, not appending anything for arg\n");
}
else
{
// If we don't have something custom to append,
// just append the arg node as an unused value.
if (newStmt == nullptr)
{
newStmt = gtNewStmt(gtUnusedValNode(argNode), callILOffset);
}

afterStmt = fgInsertStmtAfter(block, afterStmt, newStmt);
#ifdef DEBUG
if (verbose)
{
gtDispTree(afterStmt);
}
#endif // DEBUG
}
}
else if (argNode->IsBoxedValue())
{
Expand Down
1 change: 1 addition & 0 deletions src/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6415,6 +6415,7 @@ GenTreeLclFld* Compiler::gtNewLclFldNode(unsigned lnum, var_types type, unsigned
}

GenTreePtr Compiler::gtNewInlineCandidateReturnExpr(GenTreePtr inlineCandidate, var_types type)

Copy link

Choose a reason for hiding this comment

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

Revert?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure.

{
assert(GenTree::s_gtNodeSizes[GT_RET_EXPR] == TREE_NODE_SZ_LARGE);

Expand Down
4 changes: 3 additions & 1 deletion src/jit/gentree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3654,6 +3654,8 @@ struct GenTreeCall final : public GenTree
// to restore real function address and load hidden argument
// as the first argument for calli. It is CoreRT replacement for instantiating
// stubs, because executable code cannot be generated at runtime.
#define GTF_CALL_M_HELPER_SPECIAL_DCE 0x00020000 // GT_CALL -- this helper call can be removed if it is part of a comma and
// the comma result is unused.

// clang-format on

Expand Down Expand Up @@ -4997,7 +4999,7 @@ struct GenTreeStoreInd : public GenTreeIndir

struct GenTreeRetExpr : public GenTree
{
GenTreePtr gtInlineCandidate;
GenTree* gtInlineCandidate;

CORINFO_CLASS_HANDLE gtRetClsHnd;

Expand Down
Loading