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

Initial version of class profiling for PGO #45133

Merged
merged 10 commits into from
Nov 25, 2020
Merged
1 change: 1 addition & 0 deletions eng/pipelines/common/templates/runtimes/run-test-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ jobs:
- jitobjectstackallocation
- jitpgo
- jitpgo_inline
- jitpgo_classes
${{ if in(parameters.testGroup, 'ilasm') }}:
scenarios:
- ilasmroundtrip
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -993,6 +993,15 @@ HRESULT getMethodBlockCounts(CORINFO_METHOD_HANDLE ftnHnd,
BlockCounts** pBlockCounts,
UINT32 * pNumRuns);

// Get the likely implementing class for a virtual call or interface call made by ftnHnd
Copy link
Member

Choose a reason for hiding this comment

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

This file should be "almost diffable" with icorjitinfo.h, so add ClassProfile struct as well

Copy link
Member Author

Choose a reason for hiding this comment

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

Commented out, like BlockCounts is ?

Copy link
Member

Choose a reason for hiding this comment

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

Commented out, like BlockCounts is ?

If necessary. I can't remember why BlockCounts is commented out; if it's required or not.

Copy link
Member

Choose a reason for hiding this comment

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

I guess if #45044 is merged, it will be unnecessary.

// at the indicated IL offset. baseHnd is the interface class or base class for the method
// being called.
CORINFO_CLASS_HANDLE getLikelyClass(CORINFO_METHOD_HANDLE ftnHnd,
CORINFO_CLASS_HANDLE baseHnd,
UINT32 ilOffset,
UINT32* pLikelihood,
UINT32* pNumberOfCases);

// Associates a native call site, identified by its offset in the native code stream, with
// the signature information and method handle the JIT used to lay out the call site. If
// the call site has no signature information (e.g. a helper call) or has no method handle
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/ToolBox/superpmi/superpmi-shared/lwmlist.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ LWM(GetJitFlags, DWORD, DD)
LWM(GetJitTimeLogFilename, DWORD, DWORD)
LWM(GetJustMyCodeHandle, DWORDLONG, DLDL)
LWM(GetLazyStringLiteralHelper, DWORDLONG, DWORD)
LWM(GetLikelyClass, Agnostic_GetLikelyClass, Agnostic_GetLikelyClassResult)
LWM(GetLocationOfThisType, DWORDLONG, Agnostic_CORINFO_LOOKUP_KIND)
LWM(GetMethodAttribs, DWORDLONG, DWORD)
LWM(GetMethodClass, DWORDLONG, DWORDLONG)
Expand Down
42 changes: 42 additions & 0 deletions src/coreclr/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5265,6 +5265,48 @@ HRESULT MethodContext::repGetMethodBlockCounts(CORINFO_METHOD_HANDLE ftnH
return result;
}

void MethodContext::recGetLikelyClass(CORINFO_METHOD_HANDLE ftnHnd, CORINFO_CLASS_HANDLE baseHnd, UINT32 ilOffset, CORINFO_CLASS_HANDLE result, UINT32* pLikelihood, UINT32* pNumberOfClasses)
{
if (GetLikelyClass == nullptr)
GetLikelyClass = new LightWeightMap<Agnostic_GetLikelyClass, Agnostic_GetLikelyClassResult>();

Agnostic_GetLikelyClass key;
ZeroMemory(&key, sizeof(Agnostic_GetLikelyClass));

key.ftnHnd = (DWORDLONG) ftnHnd;
key.baseHnd = (DWORDLONG) baseHnd;
key.ilOffset = (DWORD) ilOffset;

Agnostic_GetLikelyClassResult value;
ZeroMemory(&value, sizeof(Agnostic_GetLikelyClassResult));
value.classHnd = (DWORDLONG) result;
value.likelihood = *pLikelihood;
value.numberOfClasses = *pNumberOfClasses;

GetLikelyClass->Add(key, value);
DEBUG_REC(dmpGetLikelyClass(key, value));
}
void MethodContext::dmpGetLikelyClass(const Agnostic_GetLikelyClass& key, const Agnostic_GetLikelyClassResult& value)
{
printf("GetLikelyClass key ftn-%016llX base-%016llX il-%u, class-%016llX likelihood-%u numberOfClasses-%u",
key.ftnHnd, key.baseHnd, key.ilOffset, value.classHnd, value.likelihood, value.numberOfClasses);
}
CORINFO_CLASS_HANDLE MethodContext::repGetLikelyClass(CORINFO_METHOD_HANDLE ftnHnd, CORINFO_CLASS_HANDLE baseHnd, UINT32 ilOffset, UINT32* pLikelihood, UINT32* pNumberOfClasses)
{
Agnostic_GetLikelyClass key;
ZeroMemory(&key, sizeof(Agnostic_GetLikelyClass));
key.ftnHnd = (DWORDLONG) ftnHnd;
key.baseHnd = (DWORDLONG) baseHnd;
key.ilOffset = (DWORD) ilOffset;

Agnostic_GetLikelyClassResult value = GetLikelyClass->Get(key);
DEBUG_REP(dmpGetLikelyClass(key, value));

*pLikelihood = value.likelihood;
*pNumberOfClasses = value.numberOfClasses;
return (CORINFO_CLASS_HANDLE) value.classHnd;
}

void MethodContext::recMergeClasses(CORINFO_CLASS_HANDLE cls1, CORINFO_CLASS_HANDLE cls2, CORINFO_CLASS_HANDLE result)
{
if (MergeClasses == nullptr)
Expand Down
22 changes: 21 additions & 1 deletion src/coreclr/src/ToolBox/superpmi/superpmi-shared/methodcontext.h
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,21 @@ class MethodContext
DWORD numRuns;
DWORD result;
};

struct Agnostic_GetLikelyClass
{
DWORDLONG ftnHnd;
DWORDLONG baseHnd;
DWORD ilOffset;
};

struct Agnostic_GetLikelyClassResult
{
DWORDLONG classHnd;
DWORD likelihood;
DWORD numberOfClasses;
};

struct Agnostic_GetProfilingHandle
{
DWORD bHookFunction;
Expand Down Expand Up @@ -1193,6 +1208,10 @@ class MethodContext
ICorJitInfo::BlockCounts** pBlockCounts,
UINT32 * pNumRuns);

void recGetLikelyClass(CORINFO_METHOD_HANDLE ftnHnd, CORINFO_CLASS_HANDLE baseHnd, UINT32 ilOffset, CORINFO_CLASS_HANDLE classHnd, UINT32* pLikelihood, UINT32* pNumberOfClasses);
void dmpGetLikelyClass(const Agnostic_GetLikelyClass& key, const Agnostic_GetLikelyClassResult& value);
CORINFO_CLASS_HANDLE repGetLikelyClass(CORINFO_METHOD_HANDLE ftnHnd, CORINFO_CLASS_HANDLE baseHnd, UINT32 ilOffset, UINT32* pLikelihood, UINT32* pNumberOfClasses);

void recMergeClasses(CORINFO_CLASS_HANDLE cls1, CORINFO_CLASS_HANDLE cls2, CORINFO_CLASS_HANDLE result);
void dmpMergeClasses(DLDL key, DWORDLONG value);
CORINFO_CLASS_HANDLE repMergeClasses(CORINFO_CLASS_HANDLE cls1, CORINFO_CLASS_HANDLE cls2);
Expand Down Expand Up @@ -1359,7 +1378,7 @@ class MethodContext
};

// ********************* Please keep this up-to-date to ease adding more ***************
// Highest packet number: 181
// Highest packet number: 182
// *************************************************************************************
enum mcPackets
{
Expand Down Expand Up @@ -1458,6 +1477,7 @@ enum mcPackets
Packet_GetJitFlags = 154, // Added 2/3/2016
Packet_GetJitTimeLogFilename = 67,
Packet_GetJustMyCodeHandle = 68,
Packet_GetLikelyClass = 182, // Added 9/27/2020
Packet_GetLocationOfThisType = 69,
Packet_GetMethodAttribs = 70,
Packet_GetMethodClass = 71,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2049,6 +2049,21 @@ HRESULT interceptor_ICJI::getMethodBlockCounts(CORINFO_METHOD_HANDLE ftnHnd,
return temp;
}

// Get the likely implementing class for a virtual call or interface call made by ftnHnd
// at the indicated IL offset. baseHnd is the interface class or base class for the method
// being called.
CORINFO_CLASS_HANDLE interceptor_ICJI::getLikelyClass(CORINFO_METHOD_HANDLE ftnHnd,
CORINFO_CLASS_HANDLE baseHnd,
UINT32 ilOffset,
UINT32* pLikelihood,
UINT32* pNumberOfClasses)
{
mc->cr->AddCall("getLikelyClass");
CORINFO_CLASS_HANDLE result = original_ICorJitInfo->getLikelyClass(ftnHnd, baseHnd, ilOffset, pLikelihood, pNumberOfClasses);
mc->recGetLikelyClass(ftnHnd, baseHnd, ilOffset, result, pLikelihood, pNumberOfClasses);
return result;
}

// Associates a native call site, identified by its offset in the native code stream, with
// the signature information and method handle the JIT used to lay out the call site. If
// the call site has no signature information (e.g. a helper call) or has no method handle
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1623,6 +1623,19 @@ HRESULT interceptor_ICJI::getMethodBlockCounts(CORINFO_METHOD_HANDLE ftnHnd,
return original_ICorJitInfo->getMethodBlockCounts(ftnHnd, pCount, pBlockCounts, pNumRuns);
}

// Get the likely implementing class for a virtual call or interface call made by ftnHnd
// at the indicated IL offset. baseHnd is the interface class or base class for the method
// being called.
CORINFO_CLASS_HANDLE interceptor_ICJI::getLikelyClass(CORINFO_METHOD_HANDLE ftnHnd,
CORINFO_CLASS_HANDLE baseHnd,
UINT32 ilOffset,
UINT32* pLikelihood,
UINT32* pNumberOfClasses)
{
mcs->AddCall("getLikelyClass");
return original_ICorJitInfo->getLikelyClass(ftnHnd, baseHnd, ilOffset, pLikelihood, pNumberOfClasses);
}

// Associates a native call site, identified by its offset in the native code stream, with
// the signature information and method handle the JIT used to lay out the call site. If
// the call site has no signature information (e.g. a helper call) or has no method handle
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1441,6 +1441,18 @@ HRESULT interceptor_ICJI::getMethodBlockCounts(CORINFO_METHOD_HANDLE ftnHnd,
return original_ICorJitInfo->getMethodBlockCounts(ftnHnd, pCount, pBlockCounts, pNumRuns);
}

// Get the likely implementing class for a virtual call or interface call made by ftnHnd
// at the indicated IL offset. baseHnd is the interface class or base class for the method
// being called.
CORINFO_CLASS_HANDLE interceptor_ICJI::getLikelyClass(CORINFO_METHOD_HANDLE ftnHnd,
CORINFO_CLASS_HANDLE baseHnd,
UINT32 ilOffset,
UINT32* pLikelihood,
UINT32* pNumberOfClasses)
{
return original_ICorJitInfo->getLikelyClass(ftnHnd, baseHnd, ilOffset, pLikelihood, pNumberOfClasses);
}

// Associates a native call site, identified by its offset in the native code stream, with
// the signature information and method handle the JIT used to lay out the call site. If
// the call site has no signature information (e.g. a helper call) or has no method handle
Expand Down
13 changes: 13 additions & 0 deletions src/coreclr/src/ToolBox/superpmi/superpmi/icorjitinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1805,6 +1805,19 @@ HRESULT MyICJI::getMethodBlockCounts(CORINFO_METHOD_HANDLE ftnHnd,
return jitInstance->mc->repGetMethodBlockCounts(ftnHnd, pCount, pBlockCounts, pNumRuns);
}

// Get the likely implementing class for a virtual call or interface call made by ftnHnd
// at the indicated IL offset. baseHnd is the interface class or base class for the method
// being called.
CORINFO_CLASS_HANDLE MyICJI::getLikelyClass(CORINFO_METHOD_HANDLE ftnHnd,
CORINFO_CLASS_HANDLE baseHnd,
UINT32 ilOffset,
UINT32* pLikelihood,
UINT32* pNumberOfClasses)
{
jitInstance->mc->cr->AddCall("getLikelyClass");
return jitInstance->mc->repGetLikelyClass(ftnHnd, baseHnd, ilOffset, pLikelihood, pNumberOfClasses);
}

// Associates a native call site, identified by its offset in the native code stream, with
// the signature information and method handle the JIT used to lay out the call site. If
// the call site has no signature information (e.g. a helper call) or has no method handle
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,7 @@ enum CorInfoHelpFunc
CORINFO_HELP_STACK_PROBE, // Probes each page of the allocated stack frame

CORINFO_HELP_PATCHPOINT, // Notify runtime that code has reached a patchpoint
CORINFO_HELP_CLASSPROFILE, // Update class profile for a call site
Copy link
Member

Choose a reason for hiding this comment

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

You need to change the JIT-EE GUID


CORINFO_HELP_COUNT,
};
Expand Down
19 changes: 19 additions & 0 deletions src/coreclr/src/inc/corjit.h
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,14 @@ class ICorJitInfo : public ICorDynamicInfo
UINT32 ExecutionCount;
};

struct ClassProfile
{
enum { SIZE = 8, SAMPLE_INTERVAL = 32 };
Copy link
Member

Choose a reason for hiding this comment

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

It would be helpful to comment this structure and its fields. E.g., what is SAMPLE_INTERVAL? What is ClassProfile used for?

Is UINT32 big enough for Count (and ExecutionCount above)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add some description.

For count overflow I currently don't have great answers; it should be rare (we'd need methods that spent a lot of cycles in Tier0) but obviously it's not impossible. Right now overflow won't cause any crashes but it might lead to nonsensical profile data.

UINT32 ILOffset;
Copy link
Member

Choose a reason for hiding this comment

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

Looks like pgo.cpp (presumably elsewhere) has some "magic" code related to this: "(classProfile->ILOffset & 0x40000000)". Should that be specified here (as documentation and code) and the magic code use some member function to do this check?

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, will do. It's currently done so that we can allocate class profiles and block counts as one big chunk without telling the runtime how many of either exist. The high bit tells us this is a class profile, and the next bit which kind.

This magic will go away once we get more of the runtime side implemented and can specify a more flexible "schema" for the profile data.

UINT32 Count;
CORINFO_CLASS_HANDLE ClassTable[SIZE];
};

// allocate a basic block profile buffer where execution counts will be stored
// for jitted basic blocks.
virtual HRESULT allocMethodBlockCounts (
Expand All @@ -267,6 +275,17 @@ class ICorJitInfo : public ICorDynamicInfo
UINT32 * pNumRuns // pointer to the total number of profile scenarios run
) = 0;

// Get the likely implementing class for a virtual call or interface call made by ftnHnd
// at the indicated IL offset. baseHnd is the interface class or base class for the method
// being called.
virtual CORINFO_CLASS_HANDLE getLikelyClass(
CORINFO_METHOD_HANDLE ftnHnd,
CORINFO_CLASS_HANDLE baseHnd,
UINT32 ilOffset,
UINT32 * pLikelihood, // estimated likelihood of the class (0...100)
Copy link
Member

Choose a reason for hiding this comment

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

Clarify this is a decimal percentage? i.e. 100 = 100% certainty.

Copy link
Member

Choose a reason for hiding this comment

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

Clarify these are OUT params

UINT32 * pNumberOfClasses // estimated number of possible classes
) = 0;

// Associates a native call site, identified by its offset in the native code stream, with
// the signature information and method handle the JIT used to lay out the call site. If
// the call site has no signature information (e.g. a helper call) or has no method handle
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/inc/jithelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@
#endif

JITHELPER(CORINFO_HELP_PATCHPOINT, JIT_Patchpoint, CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_CLASSPROFILE, JIT_ClassProfile, CORINFO_HELP_SIG_REG_ONLY)

#undef JITHELPER
#undef DYNAMICJITHELPER
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/src/jit/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,7 @@ struct BasicBlock : private LIR::Range

#define BBF_BACKWARD_JUMP_TARGET MAKE_BBFLAG(36) // Block is a target of a backward jump
#define BBF_PATCHPOINT MAKE_BBFLAG(37) // Block is a patchpoint
#define BBF_HAS_VIRTUAL_CALL MAKE_BBFLAG(38) // BB contains a call needing a class profile
AndyAyersMS marked this conversation as resolved.
Show resolved Hide resolved

// clang-format on

Expand Down Expand Up @@ -492,7 +493,7 @@ struct BasicBlock : private LIR::Range
#define BBF_SPLIT_GAINED \
(BBF_DONT_REMOVE | BBF_HAS_LABEL | BBF_HAS_JMP | BBF_BACKWARD_JUMP | BBF_HAS_IDX_LEN | BBF_HAS_NEWARRAY | \
BBF_PROF_WEIGHT | BBF_HAS_NEWOBJ | BBF_KEEP_BBJ_ALWAYS | BBF_CLONED_FINALLY_END | BBF_HAS_NULLCHECK | \
BBF_HAS_VTABREF)
BBF_HAS_VTABREF | BBF_HAS_VIRTUAL_CALL)

#ifndef __GNUC__ // GCC doesn't like C_ASSERT at global scope
static_assert_no_msg((BBF_SPLIT_NONEXIST & BBF_SPLIT_LOST) == 0);
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5982,6 +5982,7 @@ int Compiler::compCompileHelper(CORINFO_MODULE_HANDLE classPtr,
info.compNativeCodeSize = 0;
info.compTotalHotCodeSize = 0;
info.compTotalColdCodeSize = 0;
info.compClassProbeCount = 0;

compHasBackwardJump = false;

Expand Down
11 changes: 9 additions & 2 deletions src/coreclr/src/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -3779,7 +3779,8 @@ class Compiler
CORINFO_CONTEXT_HANDLE* contextHandle,
CORINFO_CONTEXT_HANDLE* exactContextHandle,
bool isLateDevirtualization,
bool isExplicitTailCall);
bool isExplicitTailCall,
IL_OFFSETX ilOffset = BAD_IL_OFFSET);

//=========================================================================
// PROTECTED
Expand Down Expand Up @@ -6697,6 +6698,7 @@ class Compiler
#define OMF_HAS_EXPRUNTIMELOOKUP 0x00000100 // Method contains a runtime lookup to an expandable dictionary.
#define OMF_HAS_PATCHPOINT 0x00000200 // Method contains patchpoints
#define OMF_NEEDS_GCPOLLS 0x00000400 // Method needs GC polls
#define OMF_HAS_VIRTUAL_CALL 0x00000800 // Method contains call that needs a class profile

bool doesMethodHaveFatPointer()
{
Expand Down Expand Up @@ -6734,7 +6736,8 @@ class Compiler
CORINFO_METHOD_HANDLE methodHandle,
CORINFO_CLASS_HANDLE classHandle,
unsigned methodAttr,
unsigned classAttr);
unsigned classAttr,
unsigned likelihood);

bool doesMethodHaveExpRuntimeLookup()
{
Expand Down Expand Up @@ -9299,6 +9302,10 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
#define CPU_ARM64 0x0400 // The generic ARM64 CPU

unsigned genCPU; // What CPU are we running on

// Number of class profile probes in this method
unsigned compClassProbeCount;

} info;

// Returns true if the method being compiled returns a non-void and non-struct value.
Expand Down
Loading