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

Guarded Devirt: multiple type checks #59604

Closed
wants to merge 29 commits into from
Closed

Guarded Devirt: multiple type checks #59604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
c1c9626
GuardedDevirt: multiple type checks
EgorBo Sep 25, 2021
f58f930
Fix weights
EgorBo Sep 25, 2021
ad6daec
remove redundant block
EgorBo Sep 25, 2021
7d7bbac
fix build
EgorBo Sep 25, 2021
c629fa1
Fix weights
EgorBo Sep 25, 2021
c2cbe9d
jit-format
EgorBo Sep 25, 2021
7495913
Remove gtGuardedDevirtualizationCandidateInfo field
EgorBo Sep 25, 2021
781c2e7
fix inlining for void methods
EgorBo Oct 2, 2021
07e20cc
Fix non-void return type
EgorBo Oct 2, 2021
bcf7b4f
fix assert
EgorBo Oct 2, 2021
81c94af
Disable multi-checks (for CI)
EgorBo Oct 2, 2021
c048c64
fix more asserts
EgorBo Oct 3, 2021
6a3bdff
Merge branch 'main' of https://github.com/dotnet/runtime into gdv-mul…
EgorBo Oct 3, 2021
37ca01f
Fix all existing issues
EgorBo Oct 3, 2021
756dfdb
disable non-void methods for now
EgorBo Oct 3, 2021
a76b01b
Disable GDV-Chain if multi-check is enabled
EgorBo Oct 3, 2021
80c3399
Formatting
EgorBo Oct 3, 2021
b9a1a05
Clean up
EgorBo Oct 3, 2021
db50163
Clean up
EgorBo Oct 3, 2021
8e70304
enable chain opt for monomorphic calls
EgorBo Oct 4, 2021
158b7d4
enable chain opt for monomorphic calls
EgorBo Oct 4, 2021
e6c989a
Update src/coreclr/jit/importer.cpp
EgorBo Oct 4, 2021
616c97f
Address feedback
EgorBo Oct 4, 2021
720be25
Merge branch 'gdv-multiple-1' of https://github.com/EgorBo/runtime-1 …
EgorBo Oct 4, 2021
710c149
Update indirectcalltransformer.cpp
EgorBo Oct 4, 2021
3805cb3
Merge branch 'main' of https://github.com/dotnet/runtime into gdv-mul…
EgorBo Oct 4, 2021
5f68a56
Address feedback
EgorBo Oct 5, 2021
1043290
Address feedback
EgorBo Oct 5, 2021
910c5c9
Address feedback
EgorBo Oct 5, 2021
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
8 changes: 4 additions & 4 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4835,7 +4835,8 @@ class Compiler
unsigned methAttr,
CORINFO_CONTEXT_HANDLE exactContextHnd,
InlineCandidateInfo** ppInlineCandidateInfo,
InlineResult* inlineResult);
InlineResult* inlineResult,
UINT8 candidateId);

void impInlineRecordArgInfo(InlineInfo* pInlineInfo,
GenTree* curArgVal,
Expand All @@ -4860,9 +4861,10 @@ class Compiler
bool exactContextNeedsRuntimeLookup,
CORINFO_CALL_INFO* callInfo);

void impMarkInlineCandidateHelper(GenTreeCall* call,
bool impMarkInlineCandidateHelper(GenTreeCall* call,
CORINFO_CONTEXT_HANDLE exactContextHnd,
bool exactContextNeedsRuntimeLookup,
UINT8 candidateIndex,
CORINFO_CALL_INFO* callInfo);

bool impTailCallRetTypeCompatible(bool allowWidening,
Expand Down Expand Up @@ -7377,8 +7379,6 @@ class Compiler
void addGuardedDevirtualizationCandidate(GenTreeCall* call,
CORINFO_METHOD_HANDLE methodHandle,
CORINFO_CLASS_HANDLE classHandle,
unsigned methodAttr,
unsigned classAttr,
unsigned likelihood);

bool doesMethodHaveExpRuntimeLookup()
Expand Down
7 changes: 4 additions & 3 deletions src/coreclr/jit/fginline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,8 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, InlineResult* inlineRe
inlineInfo.hasSIMDTypeArgLocalOrReturn = false;
#endif // FEATURE_SIMD

InlineCandidateInfo* inlineCandidateInfo = call->gtInlineCandidateInfo;
assert(call->GetGDVCandidatesCount() <= 1);
InlineCandidateInfo* inlineCandidateInfo = call->GetInlineCandidateInfo();
noway_assert(inlineCandidateInfo);
// Store the link to inlineCandidateInfo into inlineInfo
inlineInfo.inlineCandidateInfo = inlineCandidateInfo;
Expand Down Expand Up @@ -1430,10 +1431,10 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
// but may still be referenced from a GT_RET_EXPR node. We will replace GT_RET_EXPR node
// in fgUpdateInlineReturnExpressionPlaceHolder. At that time we will also update the flags
// on the basic block of GT_RET_EXPR node.
if (iciCall->gtInlineCandidateInfo->retExpr->OperGet() == GT_RET_EXPR)
if (iciCall->GetInlineCandidateInfo()->retExpr->OperGet() == GT_RET_EXPR)
{
// Save the basic block flags from the retExpr basic block.
iciCall->gtInlineCandidateInfo->retExpr->AsRetExpr()->bbFlags = pInlineInfo->retBB->bbFlags;
iciCall->GetInlineCandidateInfo()->retExpr->AsRetExpr()->bbFlags = pInlineInfo->retBB->bbFlags;
}
iciCall->ReplaceWith(pInlineInfo->retExpr, this);
}
Expand Down
89 changes: 80 additions & 9 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,76 @@ bool GenTreeCall::IsHelperCall(Compiler* compiler, unsigned helper) const
return IsHelperCall(compiler->eeFindHelper(helper));
}

//-------------------------------------------------------------------------
// SetSingleInlineCandidate: Sets (copies) inline info to the current call.
//
// Arguments:
// compiler - the compiler instance for allocations
// info - inline candidate info to set
//
// Return Value:
// Returns pointer to the copied inline info
//
InlineCandidateInfo* GenTreeCall::SetSingleInlineCandidate(Compiler* comp, const InlineCandidateInfo* info)
{
assert(info != nullptr);
assert(GetGDVCandidatesCount() == 0);
gtInlineCandidateInfo = new (comp, CMK_Inlining) InlineCandidateInfo[1]{*info};
return gtInlineCandidateInfo;
}

//-------------------------------------------------------------------------
// AddGDVInlineCandidate: Adds (copies) given inline candidate info to the list
// of GDV candidates.
//
// Arguments:
// compiler - the compiler instance for allocations
// info - inline candidate info to add
//
// Return Value:
// Returns pointer to the copied inline info
//
InlineCandidateInfo* GenTreeCall::AddGDVInlineCandidate(Compiler* comp, const InlineCandidateInfo* info)
{
assert(IsGuardedDevirtualizationCandidate());
const UINT8 maxCandidates = (UINT8)JitConfig.JitGuardedDevirtualizationCheckCount();
if (GetGDVCandidatesCount() == 0)
{
// In >90% cases we deal with monomorphic calls so let's avoid allocating too much.
SetSingleInlineCandidate(comp, info);
SetGDVCandidatesCount(1);
}
else if (GetGDVCandidatesCount() == 1)
{
// Non-monomorphic case - re-alloc
const InlineCandidateInfo firstInfo = gtInlineCandidateInfo[0];
gtInlineCandidateInfo = new (comp, CMK_Inlining) InlineCandidateInfo[maxCandidates]{firstInfo, *info};
SetGDVCandidatesCount(2);
}
else
{
gtInlineCandidateInfo[GetGDVCandidatesCount()] = *info;
SetGDVCandidatesCount(GetGDVCandidatesCount() + 1);
}
assert(GetGDVCandidatesCount() <= maxCandidates);
return GetInlineCandidateInfo(GetGDVCandidatesCount() - 1);
}

//-------------------------------------------------------------------------
// GetInlineCandidateInfo: Returns an inline candidate for given index.
//
// Arguments:
// index - 0 or index of a candidate for GDV
//
// Return Value:
// Returns an inline candidate for given index.
//
InlineCandidateInfo* GenTreeCall::GetInlineCandidateInfo(UINT8 index) const
{
assert(max(1, GetGDVCandidatesCount()) > index);
return gtInlineCandidateInfo + index;
}

//------------------------------------------------------------------------
// GenTreeCall::ReplaceCallOperand:
// Replaces a given operand to a call node and updates the call
Expand Down Expand Up @@ -6415,17 +6485,18 @@ GenTreeCall* Compiler::gtNewCallNode(
node->gtRetClsHnd = nullptr;
node->gtControlExpr = nullptr;
node->gtCallMoreFlags = GTF_CALL_M_EMPTY;
node->gtCallLateArgs = nullptr;
node->gtReturnType = type;
node->SetGDVCandidatesCount(0);

if (callType == CT_INDIRECT)
{
node->gtCallCookie = nullptr;
}
else
{
node->gtInlineCandidateInfo = nullptr;
node->ClearInlineInfo();
}
node->gtCallLateArgs = nullptr;
node->gtReturnType = type;

#ifdef FEATURE_READYTORUN
node->gtEntryPoint.addr = nullptr;
Expand Down Expand Up @@ -8308,8 +8379,8 @@ GenTreeCall* Compiler::gtCloneExprCallHelper(GenTreeCall* tree,
}
else
{
copy->gtCallMethHnd = tree->gtCallMethHnd;
copy->gtInlineCandidateInfo = nullptr;
copy->gtCallMethHnd = tree->gtCallMethHnd;
copy->ClearInlineInfo();
}

if (tree->fgArgInfo)
Expand Down Expand Up @@ -12122,10 +12193,10 @@ void Compiler::gtDispTree(GenTree* tree,
printf(" (FramesRoot last use)");
}

if (((call->gtFlags & GTF_CALL_INLINE_CANDIDATE) != 0) && (call->gtInlineCandidateInfo != nullptr) &&
(call->gtInlineCandidateInfo->exactContextHnd != nullptr))
if (((call->gtFlags & GTF_CALL_INLINE_CANDIDATE) != 0) && (call->GetInlineCandidateInfo() != nullptr) &&
(call->GetInlineCandidateInfo()->exactContextHnd != nullptr))
{
printf(" (exactContextHnd=0x%p)", dspPtr(call->gtInlineCandidateInfo->exactContextHnd));
printf(" (exactContextHnd=0x%p)", dspPtr(call->GetInlineCandidateInfo()->exactContextHnd));
}

gtDispCommonEndLine(tree);
Expand Down Expand Up @@ -17774,7 +17845,7 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b
{
// For inline candidates, we've already cached the return
// type class handle in the inline info.
InlineCandidateInfo* inlInfo = call->gtInlineCandidateInfo;
InlineCandidateInfo* inlInfo = call->GetInlineCandidateInfo();
assert(inlInfo != nullptr);

// Grab it as our first cut at a return type.
Expand Down
32 changes: 28 additions & 4 deletions src/coreclr/jit/gentree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4710,6 +4710,7 @@ struct GenTreeCall final : public GenTree
void ClearGuardedDevirtualizationCandidate()
{
gtCallMoreFlags &= ~GTF_CALL_M_GUARDED_DEVIRT;
SetGDVCandidatesCount(0);
}

void SetGuardedDevirtualizationCandidate()
Expand Down Expand Up @@ -4765,14 +4766,25 @@ struct GenTreeCall final : public GenTree
// only used for CALLI unmanaged calls (CT_INDIRECT)
GenTree* gtCallCookie;
// gtInlineCandidateInfo is only used when inlining methods
InlineCandidateInfo* gtInlineCandidateInfo;
GuardedDevirtualizationCandidateInfo* gtGuardedDevirtualizationCandidateInfo;
ClassProfileCandidateInfo* gtClassProfileCandidateInfo;
void* gtStubCallStubAddr; // GTF_CALL_VIRT_STUB - these are never inlined
InlineCandidateInfo* gtInlineCandidateInfo;
ClassProfileCandidateInfo* gtClassProfileCandidateInfo;
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
void* gtStubCallStubAddr; // GTF_CALL_VIRT_STUB - these are never inlined
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
};

void ClearInlineInfo()
{
SetGDVCandidatesCount(0);
gtInlineCandidateInfo = nullptr;
}

InlineCandidateInfo* SetSingleInlineCandidate(Compiler* comp, const InlineCandidateInfo* info);

InlineCandidateInfo* AddGDVInlineCandidate(Compiler* comp, const InlineCandidateInfo* info);

InlineCandidateInfo* GetInlineCandidateInfo(UINT8 index = 0) const;

// expression evaluated after args are placed which determines the control target
GenTree* gtControlExpr;

Expand All @@ -4795,6 +4807,18 @@ struct GenTreeCall final : public GenTree
IL_OFFSET gtRawILOffset;
#endif // defined(DEBUG) || defined(INLINE_DATA)

UINT8 gtGDVCandidatesCount;
Copy link
Member

Choose a reason for hiding this comment

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

Does this make a call node bigger?

If we have room for this, do we have enough room to make the union above a tagged union?

Copy link
Member Author

@EgorBo EgorBo Oct 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@AndyAyersMS Not sure I follow, here is the current win-x64 layout (before my changes) in Release /d1reportSingleClassLayoutGenTreeCall:

4>class GenTreeCall	size(144):
4>	+---
4> 0	| +--- (base class GenTree)
4> 0	| | genTreeOps gtOper
4> 1	| | var_types gtType
4> 2	| | gtCSEnum
4> 3	| | gtLIRFlags
4> 4	| | AssertionInfo gtAssertionInfo
4> 6	| | _gtCostEx
4> 7	| | _gtCostSz
4> 8	| | _gtRegNum
4>  	| | <alignment member> (size=3)
4>12	| | GenTreeFlags gtFlags
4>16	| | ValueNumPair gtVNPair
4>24	| | gtRsvdRegs
4>  	| | <alignment member> (size=4)
4>32	| | gtNext
4>40	| | gtPrev
4>	| +---
4>48	| gtCallThisArg
4>56	| gtCallArgs
4>64	| gtCallLateArgs
4>72	| fgArgInfo
4>80	| tailCallInfo
4>80	| CorInfoCallConvExtension unmgdCallConv
4>88	| GenTreeCallFlags gtCallMoreFlags
4>92.	| gtCallType (bitstart=0,nbits=3)
4>92.	| gtReturnType (bitstart=3,nbits=5)
4>  	| <alignment member> (size=3)
4>96	| gtRetClsHnd
4>104	| gtCallCookie
4>104	| gtInlineCandidateInfo
4>104	| gtClassProfileCandidateInfo
4>104	| gtStubCallStubAddr
4>104	| compileTimeHelperArgumentHandle
4>104	| gtDirectCallAddress
4>112	| gtControlExpr
4>120	| gtCallMethHnd
4>120	| gtCallAddr
4>128	| CORINFO_CONST_LOOKUP gtEntryPoint

in theory we can fit the count into those spare 3 bits? (means maxvalue is 7) if the size matters that much - not sure about impact on other archs/os


UINT8 GetGDVCandidatesCount() const
{
return gtGDVCandidatesCount;
}

void SetGDVCandidatesCount(UINT8 count)
{
gtGDVCandidatesCount = count;
}

bool IsHelperCall() const
{
return gtCallType == CT_HELPER;
Expand Down
Loading