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

JIT: save generics context for late devirtualization #63420

Merged
merged 3 commits into from
Jan 6, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
11 changes: 9 additions & 2 deletions src/coreclr/jit/fginline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -717,11 +717,18 @@ Compiler::fgWalkResult Compiler::fgLateDevirtualization(GenTree** pTree, fgWalkD
}
#endif // DEBUG

CORINFO_CONTEXT_HANDLE context = nullptr;
CORINFO_METHOD_HANDLE method = call->gtCallMethHnd;
unsigned methodFlags = 0;
CORINFO_CONTEXT_HANDLE context = nullptr;
const bool isLateDevirtualization = true;
bool explicitTailCall = (call->AsCall()->gtCallMoreFlags & GTF_CALL_M_EXPLICIT_TAILCALL) != 0;
const bool explicitTailCall = (call->AsCall()->gtCallMoreFlags & GTF_CALL_M_EXPLICIT_TAILCALL) != 0;
Copy link
Member

Choose a reason for hiding this comment

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

just a nit: there is IsTailPrefixedCall()

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, will change it over. Also, we already know call is GenTreeCall*.


if ((call->gtCallMoreFlags & GTF_CALL_M_LATE_DEVIRT) != 0)
{
context = call->gtLateDevirtualizationInfo->exactContextHnd;
call->gtLateDevirtualizationInfo = nullptr;
}

comp->impDevirtualizeCall(call, nullptr, &method, &methodFlags, &context, nullptr, isLateDevirtualization,
explicitTailCall);
*madeChanges = true;
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6143,7 +6143,7 @@ GenTree* Compiler::gtNewInlineCandidateReturnExpr(GenTree* inlineCandidate, var_

if (varTypeIsStruct(inlineCandidate) && !inlineCandidate->OperIsBlkOp())
{
node->AsRetExpr()->gtRetClsHnd = gtGetStructHandle(inlineCandidate);
node->gtRetClsHnd = gtGetStructHandle(inlineCandidate);
}

// GT_RET_EXPR node eventually might be bashed back to GT_CALL (when inlining is aborted for example).
Expand Down Expand Up @@ -7798,7 +7798,7 @@ GenTreeCall* Compiler::gtCloneExprCallHelper(GenTreeCall* tree,
else
{
copy->gtCallMethHnd = tree->gtCallMethHnd;
copy->gtInlineCandidateInfo = nullptr;
copy->gtInlineCandidateInfo = tree->gtInlineCandidateInfo;
}

copy->gtCallType = tree->gtCallType;
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ enum BasicBlockFlags : unsigned __int64;
struct InlineCandidateInfo;
struct GuardedDevirtualizationCandidateInfo;
struct ClassProfileCandidateInfo;
struct LateDevirtualizationInfo;

typedef unsigned short AssertionIndex;

Expand Down Expand Up @@ -3760,7 +3761,7 @@ enum GenTreeCallFlags : unsigned int
GTF_CALL_M_EXP_RUNTIME_LOOKUP = 0x02000000, // this call needs to be tranformed into CFG for the dynamic dictionary expansion feature.
GTF_CALL_M_STRESS_TAILCALL = 0x04000000, // the call is NOT "tail" prefixed but GTF_CALL_M_EXPLICIT_TAILCALL was added because of tail call stress mode
GTF_CALL_M_EXPANDED_EARLY = 0x08000000, // the Virtual Call target address is expanded and placed in gtControlExpr in Morph rather than in Lower

GTF_CALL_M_LATE_DEVIRT = 0x10000000, // this call has late devirtualzation info
};

inline constexpr GenTreeCallFlags operator ~(GenTreeCallFlags a)
Expand Down Expand Up @@ -4742,7 +4743,7 @@ struct GenTreeCall final : public GenTree
InlineCandidateInfo* gtInlineCandidateInfo;
GuardedDevirtualizationCandidateInfo* gtGuardedDevirtualizationCandidateInfo;
ClassProfileCandidateInfo* gtClassProfileCandidateInfo;

LateDevirtualizationInfo* gtLateDevirtualizationInfo;
CORINFO_GENERIC_HANDLE compileTimeHelperArgumentHandle; // Used to track type handle argument of dynamic helpers
void* gtDirectCallAddress; // Used to pass direct call address between lower and codegen
};
Expand Down
16 changes: 16 additions & 0 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9950,6 +9950,22 @@ var_types Compiler::impImportCall(OPCODE opcode,
}
else
{
// If the call is virtual, and has a generics context, and is not going to have a class probe,
// record the context for possible use during late devirt.
//
// If we ever want to devirt at Tier0, and/or see issues where OSR methods under PGO lose
// important devirtualizations, we'll want to allow both a class probe and a captured context.
//
if (origCall->IsVirtual() && (origCall->gtCallType != CT_INDIRECT) && (exactContextHnd != nullptr) &&
(origCall->gtClassProfileCandidateInfo == nullptr))
{
JITDUMP("\nSaving context %p for call [%06u]\n", exactContextHnd, dspTreeID(origCall));
origCall->gtCallMoreFlags |= GTF_CALL_M_LATE_DEVIRT;
LateDevirtualizationInfo* const info = new (this, CMK_Inlining) LateDevirtualizationInfo;
Copy link
Member

Choose a reason for hiding this comment

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

Are you planning to add more fields to LateDevirtualizationInfo? because for now it looks like allocation is not needed

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe? That's what I meant by

I am deliberately leaving LateDevirtualizationInfo more general than
necessary as it may serve as a jumping-off point for enabling late inlining.

info->exactContextHnd = exactContextHnd;
origCall->gtLateDevirtualizationInfo = info;
}

if (isFatPointerCandidate)
{
// fatPointer candidates should be in statements of the form call() or var = call().
Expand Down
9 changes: 9 additions & 0 deletions src/coreclr/jit/inline.h
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,15 @@ struct InlineCandidateInfo : public GuardedDevirtualizationCandidateInfo
InlineContext* inlinersContext;
};

// LateDevirtualizationInfo
//
// Used to fill in missing contexts during late devirtualization.
//
struct LateDevirtualizationInfo
{
CORINFO_CONTEXT_HANDLE exactContextHnd;
};

// InlArgInfo describes inline candidate argument properties.

struct InlArgInfo
Expand Down