Skip to content

Commit

Permalink
Allow jit to disregard PGO; PGO changes for SPMI and MCS (#49551)
Browse files Browse the repository at this point in the history
* COMPlus_JitDisablePGO will now block the jit from using profile data. Useful
for investigations and (someday) bug triage. Also disable GDV if we're disabling PGO

* `-base_jit_option` and `-diff_jit_option` can be used to pass options to
superpmy.py asmdiffs.

* Fix SPMI file name computation to better handle cases where the directory is
a relative path.

* Enhance `mcs -jitflags` output to describe how many method contexts have
PGO data, and which kinds of data they hold. Useful for validating that an
SPMI collection done via dynamic PGO has actually collected an interesting
set of jit requests.
  • Loading branch information
AndyAyersMS authored Mar 17, 2021
1 parent 4e3deb5 commit 1ba22eb
Show file tree
Hide file tree
Showing 11 changed files with 207 additions and 53 deletions.
20 changes: 20 additions & 0 deletions src/coreclr/ToolBox/superpmi/mcs/verbjitflags.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,26 @@ int verbJitFlags::DoWork(const char* nameOfInput)
mc->repGetJitFlags(&corJitFlags, sizeof(corJitFlags));
unsigned long long rawFlags = corJitFlags.GetFlagsRaw();

// We co-opt unused flag bits to note if there's pgo data,
// and if so, what kind
//
bool hasEdgeProfile = false;
bool hasClassProfile = false;
if (mc->hasPgoData(hasEdgeProfile, hasClassProfile))
{
rawFlags |= 1ULL << (EXTRA_JIT_FLAGS::HAS_PGO);

if (hasEdgeProfile)
{
rawFlags |= 1ULL << (EXTRA_JIT_FLAGS::HAS_EDGE_PROFILE);
}

if (hasClassProfile)
{
rawFlags |= 1ULL << (EXTRA_JIT_FLAGS::HAS_CLASS_PROFILE);
}
}

int index = flagMap.GetIndex(rawFlags);
if (index == -1)
{
Expand Down
38 changes: 38 additions & 0 deletions src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6765,6 +6765,44 @@ int MethodContext::dumpMD5HashToBuffer(BYTE* pBuffer, int bufLen, char* hash, in
return m_hash.HashBuffer(pBuffer, bufLen, hash, hashLen);
}

bool MethodContext::hasPgoData(bool& hasEdgeProfile, bool& hasClassProfile)
{
hasEdgeProfile = false;
hasClassProfile = false;

// Obtain the Method Info structure for this method
CORINFO_METHOD_INFO info;
unsigned flags = 0;
repCompileMethod(&info, &flags);

if ((GetPgoInstrumentationResults != nullptr) &&
(GetPgoInstrumentationResults->GetIndex(CastHandle(info.ftn)) != -1))
{
ICorJitInfo::PgoInstrumentationSchema* schema = nullptr;
UINT32 schemaCount = 0;
BYTE* schemaData = nullptr;
HRESULT pgoHR = repGetPgoInstrumentationResults(info.ftn, &schema, &schemaCount, &schemaData);

if (SUCCEEDED(pgoHR))
{
for (UINT32 i = 0; i < schemaCount; i++)
{
hasEdgeProfile |= (schema[i].InstrumentationKind == ICorJitInfo::PgoInstrumentationKind::EdgeIntCount);
hasClassProfile |= (schema[i].InstrumentationKind == ICorJitInfo::PgoInstrumentationKind::TypeHandleHistogramCount);

if (hasEdgeProfile && hasClassProfile)
{
break;
}
}

return true;
}
}

return false;
}

MethodContext::Environment MethodContext::cloneEnvironment()
{
MethodContext::Environment env;
Expand Down
17 changes: 17 additions & 0 deletions src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,21 @@ const char* toString(CorInfoType cit);

#define METHOD_IDENTITY_INFO_SIZE 0x10000 // We assume that the METHOD_IDENTITY_INFO_SIZE will not exceed 64KB

// Special "jit flags" for noting some method context features

enum EXTRA_JIT_FLAGS
{
HAS_PGO = 63,
HAS_EDGE_PROFILE = 62,
HAS_CLASS_PROFILE = 61
};

// Asserts to catch changes in corjit flags definitions.

static_assert((int)EXTRA_JIT_FLAGS::HAS_PGO == (int)CORJIT_FLAGS::CorJitFlag::CORJIT_FLAG_UNUSED36, "Jit Flags Mismatch");
static_assert((int)EXTRA_JIT_FLAGS::HAS_EDGE_PROFILE == (int)CORJIT_FLAGS::CorJitFlag::CORJIT_FLAG_UNUSED35, "Jit Flags Mismatch");
static_assert((int)EXTRA_JIT_FLAGS::HAS_CLASS_PROFILE == (int)CORJIT_FLAGS::CorJitFlag::CORJIT_FLAG_UNUSED34, "Jit Flags Mismatch");

class MethodContext
{
public:
Expand Down Expand Up @@ -82,6 +97,8 @@ class MethodContext
int dumpMethodIdentityInfoToBuffer(char* buff, int len, bool ignoreMethodName = false, CORINFO_METHOD_INFO* optInfo = nullptr, unsigned optFlags = 0);
int dumpMethodMD5HashToBuffer(char* buff, int len, bool ignoreMethodName = false, CORINFO_METHOD_INFO* optInfo = nullptr, unsigned optFlags = 0);

bool hasPgoData(bool& hasEdgeProfile, bool& hasClassProfile);

void recGlobalContext(const MethodContext& other);

void dmpEnvironment(DWORD key, const Agnostic_Environment& value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,12 @@ std::string SpmiDumpHelper::DumpJitFlags(unsigned long long flags)

AddFlag(NO_INLINING);

// "Extra jit flag" support
//
AddFlagNumeric(HAS_PGO, EXTRA_JIT_FLAGS::HAS_PGO);
AddFlagNumeric(HAS_EDGE_PROFILE, EXTRA_JIT_FLAGS::HAS_EDGE_PROFILE);
AddFlagNumeric(HAS_CLASS_PROFILE, EXTRA_JIT_FLAGS::HAS_CLASS_PROFILE);

#undef AddFlag
#undef AddFlagNumeric

Expand Down
92 changes: 56 additions & 36 deletions src/coreclr/ToolBox/superpmi/superpmi-shared/spmiutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,58 +156,76 @@ void ReplaceIllegalCharacters(WCHAR* fileName)
// All lengths in this function exclude the terminal NULL.
WCHAR* GetResultFileName(const WCHAR* folderPath, const WCHAR* fileName, const WCHAR* extension)
{
const size_t folderPathLength = wcslen(folderPath);
const size_t fileNameLength = wcslen(fileName);
const size_t extensionLength = wcslen(extension);
const size_t maxPathLength = MAX_PATH - 50; // subtract 50 because excel doesn't like paths longer then 230.
const size_t fileNameLength = wcslen(fileName);
const size_t randomStringLength = 8;
const size_t maxPathLength = MAX_PATH - 50;
bool appendRandomString = false;

size_t fullPathLength = folderPathLength + 1 + extensionLength;
bool appendRandomString = false;
// See how long the folder part is, and start building the file path with the folder part.
//
WCHAR* fullPath = new WCHAR[MAX_PATH];
fullPath[0] = W('\0');
const size_t folderPathLength = GetFullPathNameW(folderPath, MAX_PATH, (LPWSTR)fullPath, NULL);

if (fileNameLength > 0)
{
fullPathLength += fileNameLength;
}
else
if (folderPathLength == 0)
{
fullPathLength += randomStringLength;
appendRandomString = true;
LogError("GetResultFileName - can't resolve folder path '%ws'", folderPath);
return nullptr;
}

size_t charsToDelete = 0;
// Account for the folder, directory separator and extension.
//
size_t fullPathLength = folderPathLength + 1 + extensionLength;

if (fullPathLength > maxPathLength)
// If we won't have room for a minimal file name part, bail.
//
if ((fullPathLength + randomStringLength) > maxPathLength)
{
// The path name is too long; creating the file will fail. This can happen because we use the command line,
// which for ngen includes lots of environment variables, for example.
// Shorten the file name and add a random string to the end to avoid collisions.
LogError("GetResultFileName - folder path '%ws' length + minimal file name exceeds limit %d", fullPath, maxPathLength);
return nullptr;
}

charsToDelete = fullPathLength - maxPathLength + randomStringLength;
// Now figure out the file name part.
//
const size_t maxFileNameLength = maxPathLength - fullPathLength;
size_t usableFileNameLength = 0;

if (fileNameLength >= charsToDelete)
{
appendRandomString = true;
fullPathLength = maxPathLength;
}
else
{
LogError("GetResultFileName - path to the output file is too long '%ws\\%ws.%ws(%d)'", folderPath, fileName, extension, fullPathLength);
return nullptr;
}
if (fileNameLength == 0)
{
// No file name provided. Use random string.
//
fullPathLength += randomStringLength;
appendRandomString = true;
}
else if (fileNameLength < maxFileNameLength)
{
// Reasonable length file name, use as is.
//
usableFileNameLength = fileNameLength;
fullPathLength += fileNameLength;
appendRandomString = false;
}
else
{
// Overly long file name, truncate and add random string.
//
usableFileNameLength = maxFileNameLength - randomStringLength;
fullPathLength += maxFileNameLength;
appendRandomString = true;
}

WCHAR* fullPath = new WCHAR[fullPathLength + 1];
fullPath[0] = W('\0');
wcsncat_s(fullPath, fullPathLength + 1, folderPath, folderPathLength);
// Append the file name part
//
wcsncat_s(fullPath, fullPathLength + 1, DIRECTORY_SEPARATOR_STR_W, 1);
wcsncat_s(fullPath, fullPathLength + 1, fileName, usableFileNameLength);

if (fileNameLength > charsToDelete)
{
wcsncat_s(fullPath, fullPathLength + 1, fileName, fileNameLength - charsToDelete);
ReplaceIllegalCharacters(fullPath + folderPathLength + 1);
}
// Clean up anything in the file part that can't be in a file name.
//
ReplaceIllegalCharacters(fullPath + folderPathLength + 1);

// Append random string, if we're using it.
//
if (appendRandomString)
{
unsigned randomNumber = 0;
Expand All @@ -223,6 +241,8 @@ WCHAR* GetResultFileName(const WCHAR* folderPath, const WCHAR* fileName, const W
wcsncat_s(fullPath, fullPathLength + 1, randomString, randomStringLength);
}

// Append extension
//
wcsncat_s(fullPath, fullPathLength + 1, extension, extensionLength);

return fullPath;
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2207,9 +2207,9 @@ void CodeGen::genGenerateMachineCode()
compiler->fgHaveValidEdgeWeights ? "valid" : "invalid", compiler->fgCalledCount);
}

if (compiler->fgProfileData_ILSizeMismatch)
if (compiler->fgPgoFailReason != nullptr)
{
printf("; discarded IBC profile data due to mismatch in ILSize\n");
printf("; %s\n", compiler->fgPgoFailReason);
}

if (compiler->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_ALT_JIT))
Expand Down
33 changes: 22 additions & 11 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2830,11 +2830,12 @@ void Compiler::compInitOptions(JitFlags* jitFlags)

// Profile data
//
fgPgoSchema = nullptr;
fgPgoData = nullptr;
fgPgoSchemaCount = 0;
fgPgoQueryResult = E_FAIL;
fgProfileData_ILSizeMismatch = false;
fgPgoSchema = nullptr;
fgPgoData = nullptr;
fgPgoSchemaCount = 0;
fgPgoQueryResult = E_FAIL;
fgPgoFailReason = nullptr;

if (jitFlags->IsSet(JitFlags::JIT_FLAG_BBOPT))
{
fgPgoQueryResult = info.compCompHnd->getPgoInstrumentationResults(info.compMethodHnd, &fgPgoSchema,
Expand All @@ -2846,12 +2847,22 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
//
// We will discard the IBC data in this case
//
if (FAILED(fgPgoQueryResult) && (fgPgoSchema != nullptr))
if (FAILED(fgPgoQueryResult))
{
fgPgoFailReason = (fgPgoSchema != nullptr) ? "No matching PGO data" : "No PGO data";
fgPgoData = nullptr;
fgPgoSchema = nullptr;
}
// Optionally, discard the profile data.
//
else if (JitConfig.JitDisablePGO() != 0)
{
fgProfileData_ILSizeMismatch = true;
fgPgoData = nullptr;
fgPgoSchema = nullptr;
fgPgoFailReason = "PGO data available, but JitDisablePGO != 0";
fgPgoQueryResult = E_FAIL;
fgPgoData = nullptr;
fgPgoSchema = nullptr;
}

#ifdef DEBUG
// A successful result implies a non-NULL fgPgoSchema
//
Expand Down Expand Up @@ -3364,9 +3375,9 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
printf("OPTIONS: optimized using profile data\n");
}

if (fgProfileData_ILSizeMismatch)
if (fgPgoFailReason != nullptr)
{
printf("OPTIONS: discarded IBC profile data due to mismatch in ILSize\n");
printf("OPTIONS: %s\n", fgPgoFailReason);
}

if (jitFlags->IsSet(JitFlags::JIT_FLAG_PREJIT))
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5636,7 +5636,7 @@ class Compiler
void fgIncorporateEdgeCounts();

public:
bool fgProfileData_ILSizeMismatch;
const char* fgPgoFailReason;
ICorJitInfo::PgoInstrumentationSchema* fgPgoSchema;
BYTE* fgPgoData;
UINT32 fgPgoSchemaCount;
Expand Down
9 changes: 9 additions & 0 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21504,6 +21504,15 @@ void Compiler::considerGuardedDevirtualization(

JITDUMP("Considering guarded devirtualization\n");

// We currently only get likely class guesses when there is PGO data. So if we've disabled
// PGO, just bail out.

if (JitConfig.JitDisablePGO() != 0)
{
JITDUMP("Not guessing for class; pgo disabled\n");
return;
}

// See if there's a likely guess for the class.
//
const unsigned likelihoodThreshold = isInterface ? 25 : 30;
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/jit/jitconfigvalues.h
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,9 @@ CONFIG_INTEGER(JitMinimalPrejitProfiling, W("JitMinimalPrejitProfiling"), 0)
CONFIG_INTEGER(JitClassProfiling, W("JitClassProfiling"), 1)
CONFIG_INTEGER(JitEdgeProfiling, W("JitEdgeProfiling"), 1)

// Profile consumption options
CONFIG_INTEGER(JitDisablePGO, W("JitDisablePGO"), 0) // Ignore pgo data

// Control when Virtual Calls are expanded
CONFIG_INTEGER(JitExpandCallsEarly, W("JitExpandCallsEarly"), 1) // Expand Call targets early (in the global morph
// phase)
Expand Down
Loading

0 comments on commit 1ba22eb

Please sign in to comment.