diff --git a/src/coreclr/ToolBox/superpmi/mcs/verbjitflags.cpp b/src/coreclr/ToolBox/superpmi/mcs/verbjitflags.cpp index f5f9d7786a745..a40707846887d 100644 --- a/src/coreclr/ToolBox/superpmi/mcs/verbjitflags.cpp +++ b/src/coreclr/ToolBox/superpmi/mcs/verbjitflags.cpp @@ -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) { diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp index 0a7da7e4b56c2..51faa23a60702 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp @@ -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; diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h index 4851d36966953..d71f8d35f3b33 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h @@ -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: @@ -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); diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/spmidumphelper.cpp b/src/coreclr/ToolBox/superpmi/superpmi-shared/spmidumphelper.cpp index 3530d369507ec..da63826e750b6 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/spmidumphelper.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/spmidumphelper.cpp @@ -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 diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/ToolBox/superpmi/superpmi-shared/spmiutil.cpp index 0d52ae35ed1ab..efbe6f13e1cd1 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/spmiutil.cpp @@ -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; @@ -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; diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index da78cb3c944d6..bb49d1ce0f1c6 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -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)) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index e1be2ce02a039..3570d37fc3386 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -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, @@ -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 // @@ -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)) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 80cded4dd8ab9..f48697168c596 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5636,7 +5636,7 @@ class Compiler void fgIncorporateEdgeCounts(); public: - bool fgProfileData_ILSizeMismatch; + const char* fgPgoFailReason; ICorJitInfo::PgoInstrumentationSchema* fgPgoSchema; BYTE* fgPgoData; UINT32 fgPgoSchemaCount; diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 026e333d2ef64..390597df7ebc3 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -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; diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index c892abe0a4b9c..d283522b2cdd4 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -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) diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py index 5146f91c154ef..b2036a98ce325 100755 --- a/src/coreclr/scripts/superpmi.py +++ b/src/coreclr/scripts/superpmi.py @@ -294,6 +294,8 @@ asm_diff_parser.add_argument("--diff_jit_dump", action="store_true", help="Generate JitDump output for diffs. Default: only generate asm, not JitDump.") asm_diff_parser.add_argument("-temp_dir", help="Specify a temporary directory used for a previous ASM diffs run (for which --skip_cleanup was used) to view the results. The replay command is skipped.") asm_diff_parser.add_argument("--gcinfo", action="store_true", help="Include GC info in disassembly (sets COMPlus_JitGCDump/COMPlus_NgenGCDump; requires instructions to be prefixed by offsets).") +asm_diff_parser.add_argument("-base_jit_option", action="append", help="Option to pass to the baselne JIT. Format is key=value, where key is the option name without leading COMPlus_...") +asm_diff_parser.add_argument("-diff_jit_option", action="append", help="Option to pass to the diff JIT. Format is key=value, where key is the option name without leading COMPlus_...") # subparser for upload upload_parser = subparsers.add_parser("upload", description=upload_description, parents=[core_root_parser, target_parser]) @@ -1721,6 +1723,22 @@ def replay_with_asm_diffs(self): altjit_asm_diffs_flags = target_flags altjit_replay_flags = target_flags + if self.coreclr_args.jitoption: + logging.warning("Ignoring -jitoption; use -base_jit_option or -diff_jit_option instead"); + + base_option_flags = [] + if self.coreclr_args.base_jit_option: + for o in self.coreclr_args.base_jit_option: + base_option_flags += "-jitoption", o + base_option_flags_for_diff_artifact = base_option_flags + + diff_option_flags = [] + diff_option_flags_for_diff_artifact = [] + if self.coreclr_args.diff_jit_option: + for o in self.coreclr_args.diff_jit_option: + diff_option_flags += "-jit2option", o + diff_option_flags_for_diff_artifact += "-jitoption", o + if self.coreclr_args.altjit: altjit_asm_diffs_flags += [ "-jitoption", "force", "AltJit=*", @@ -1770,6 +1788,8 @@ def replay_with_asm_diffs(self): "-r", os.path.join(temp_location, "repro") # Repro name, create .mc repro files ] flags += altjit_asm_diffs_flags + flags += base_option_flags + flags += diff_option_flags if not self.coreclr_args.sequential: flags += [ "-p" ] @@ -1858,7 +1878,7 @@ async def create_replay_artifacts(print_prefix, item, self, mch_file, env_vars, # as the LoadLibrary path will be relative to the current directory. with ChangeDir(self.coreclr_args.core_root): - async def create_one_artifact(jit_path: str, location: str) -> str: + async def create_one_artifact(jit_path: str, location: str, flags: list[str]) -> str: command = [self.superpmi_path] + flags + [jit_path, mch_file] item_path = os.path.join(location, "{}{}".format(item, extension)) with open(item_path, 'w') as file_handle: @@ -1871,8 +1891,8 @@ async def create_one_artifact(jit_path: str, location: str) -> str: return generated_txt # Generate diff and base JIT dumps - base_txt = await create_one_artifact(self.base_jit_path, base_location) - diff_txt = await create_one_artifact(self.diff_jit_path, diff_location) + base_txt = await create_one_artifact(self.base_jit_path, base_location, flags + base_option_flags_for_diff_artifact) + diff_txt = await create_one_artifact(self.diff_jit_path, diff_location, flags + diff_option_flags_for_diff_artifact) if base_txt != diff_txt: jit_differences_queue.put_nowait(item) @@ -3410,6 +3430,16 @@ def verify_replay_common_args(): lambda unused: True, "Unable to set diff_jit_dump.") + coreclr_args.verify(args, + "base_jit_option", + lambda unused: True, + "Unable to set base_jit_option.") + + coreclr_args.verify(args, + "diff_jit_option", + lambda unused: True, + "Unable to set diff_jit_option.") + process_base_jit_path_arg(coreclr_args) jit_in_product_location = False