From b54b76b950f62527e2b692aaf89026dcb4a623b6 Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Fri, 22 Mar 2024 12:35:58 -0700 Subject: [PATCH] Updates This includes #100154, #100160, #100123 --- src/coreclr/jit/compiler.cpp | 62 +++++++++++++++++++++++++------ src/coreclr/jit/eeinterface.cpp | 2 +- src/coreclr/jit/importercalls.cpp | 5 ++- src/coreclr/jit/inlinepolicy.cpp | 16 +++++++- src/coreclr/jit/jitconfigvalues.h | 37 ++++++++++++------ src/coreclr/jit/optimizer.cpp | 28 +++++++++++++- 6 files changed, 121 insertions(+), 29 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 4910148426cfc5..4a4c3e869ef5e1 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -2082,9 +2082,9 @@ void Compiler::compDoComponentUnitTestsOnce() // compGetJitDefaultFill: // // Return Value: -// An unsigned char value used to initizalize memory allocated by the JIT. -// The default value is taken from DOTNET_JitDefaultFill, if is not set -// the value will be 0xdd. When JitStress is active a random value based +// An unsigned char value used to initialize memory allocated by the JIT. +// The default value is taken from DOTNET_JitDefaultFill. If it is not set +// the value will be 0xdd. When JitStress is active a random value based // on the method hash is used. // // Notes: @@ -3104,28 +3104,50 @@ void Compiler::compInitOptions(JitFlags* jitFlags) #ifdef OPT_CONFIG if (opts.optRepeat) { + // Defer printing this until now, after the "START" line printed above. JITDUMP("\n*************** JitOptRepeat enabled; repetition count: %d\n\n", opts.optRepeatCount); } - else if ((JitConfig.JitEnableOptRepeat() != 0) && compStressCompile(STRESS_OPT_REPEAT, 10)) + else if (JitConfig.JitEnableOptRepeat() != 0) { - // Turn on optRepeat as part of JitStress. In this case, decide how many iterations to do, from 2 to 5, - // based on a random number seeded by the method hash. - opts.optRepeat = true; +#ifdef DEBUG + // Opt-in to JitOptRepeat based on method hash ranges. + // The default is no JitOptRepeat. + static ConfigMethodRange fJitOptRepeatRange; + fJitOptRepeatRange.EnsureInit(JitConfig.JitOptRepeatRange()); + assert(!fJitOptRepeatRange.Error()); + if (!fJitOptRepeatRange.IsEmpty() && fJitOptRepeatRange.Contains(info.compMethodHash())) + { + opts.optRepeat = true; + opts.optRepeatCount = JitConfig.JitOptRepeatCount(); - CLRRandom rng; - rng.Init(info.compMethodHash()); - opts.optRepeatCount = rng.Next(4) + 2; // generates [2..5] + JITDUMP("\n*************** JitOptRepeat enabled by JitOptRepeatRange; repetition count: %d\n\n", + opts.optRepeatCount); + } +#endif // DEBUG + + if (!opts.optRepeat && compStressCompile(STRESS_OPT_REPEAT, 10)) + { + // Turn on optRepeat as part of JitStress. In this case, decide how many iterations to do, from 2 to 5, + // based on a random number seeded by the method hash. + opts.optRepeat = true; - JITDUMP("\n*************** JitOptRepeat for stress; repetition count: %d\n\n", opts.optRepeatCount); + CLRRandom rng; + rng.Init(info.compMethodHash()); + opts.optRepeatCount = rng.Next(4) + 2; // generates [2..5] + + JITDUMP("\n*************** JitOptRepeat for stress; repetition count: %d\n\n", opts.optRepeatCount); + } } - ////////////////// TESTING +////////////////// TESTING +#if 0 if (JitConfig.JitEnableOptRepeat() != 0) { opts.optRepeat = true; opts.optRepeatCount = 4; JITDUMP("\n*************** JitOptRepeat FORCED; repetition count: %d\n\n", opts.optRepeatCount); } +#endif // 0 ////////////////// END TESTING #endif // OPT_CONFIG @@ -3624,6 +3646,14 @@ bool Compiler::compStressCompileHelper(compStressArea stressArea, unsigned weigh return false; } + // Does user allow using this STRESS_MODE through the command line? + const WCHAR* strStressModeNamesAllow = JitConfig.JitStressModeNamesAllow(); + if ((strStressModeNamesAllow != nullptr) && + (u16_strstr(strStressModeNamesAllow, s_compStressModeNamesW[stressArea]) == nullptr)) + { + return false; + } + // Does user explicitly set this STRESS_MODE through the command line? const WCHAR* strStressModeNames = JitConfig.JitStressModeNames(); if (strStressModeNames != nullptr) @@ -5123,6 +5153,14 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl ResetOptAnnotations(); RecomputeFlowGraphAnnotations(); + +#ifdef DEBUG + if (verbose) + { + printf("Trees before next JitOptRepeat iteration:\n"); + fgDispBasicBlocks(true); + } +#endif // DEBUG } } diff --git a/src/coreclr/jit/eeinterface.cpp b/src/coreclr/jit/eeinterface.cpp index d9852afb9e5373..0578dee4109ef1 100644 --- a/src/coreclr/jit/eeinterface.cpp +++ b/src/coreclr/jit/eeinterface.cpp @@ -614,5 +614,5 @@ void Compiler::eePrintObjectDescription(const char* prefix, CORINFO_OBJECT_HANDL } } - printf("%s '%s'\n", prefix, str); + printf("%s '%s'", prefix, str); } diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 2238924ab064a5..571935252e6728 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -1393,7 +1393,7 @@ var_types Compiler::impImportCall(OPCODE opcode, if (origCall->IsVirtual() && (origCall->gtCallType != CT_INDIRECT) && (exactContextHnd != nullptr) && (origCall->gtHandleHistogramProfileCandidateInfo == nullptr)) { - JITDUMP("\nSaving context %p for call [%06u]\n", exactContextHnd, dspTreeID(origCall)); + JITDUMP("\nSaving context %p for call [%06u]\n", dspPtr(exactContextHnd), dspTreeID(origCall)); origCall->gtCallMoreFlags |= GTF_CALL_M_HAS_LATE_DEVIRT_INFO; LateDevirtualizationInfo* const info = new (this, CMK_Inlining) LateDevirtualizationInfo; info->exactContextHnd = exactContextHnd; @@ -8449,8 +8449,9 @@ void Compiler::impCheckCanInline(GenTreeCall* call, return; } #endif + JITDUMP("\nCheckCanInline: fetching method info for inline candidate %s -- context %p\n", - compiler->eeGetMethodName(ftn), pParam->exactContextHnd); + compiler->eeGetMethodName(ftn), compiler->dspPtr(pParam->exactContextHnd)); if (pParam->exactContextHnd == METHOD_BEING_COMPILED_CONTEXT()) { diff --git a/src/coreclr/jit/inlinepolicy.cpp b/src/coreclr/jit/inlinepolicy.cpp index 9a4086dc5af891..d057ccd09ed0d9 100644 --- a/src/coreclr/jit/inlinepolicy.cpp +++ b/src/coreclr/jit/inlinepolicy.cpp @@ -1122,7 +1122,7 @@ void RandomPolicy::NoteInt(InlineObservation obs, int value) // methodInfo -- method info for the callee // // Notes: -// The random policy makes random decisions about profitablity. +// The random policy makes random decisions about profitability. // Generally we aspire to inline differently, not necessarily to // inline more. @@ -1131,6 +1131,20 @@ void RandomPolicy::DetermineProfitability(CORINFO_METHOD_INFO* methodInfo) assert(InlDecisionIsCandidate(m_Decision)); assert(m_Observation == InlineObservation::CALLEE_IS_DISCRETIONARY_INLINE); +#if defined(DEBUG) + + // Punt if we're inlining and we've reached the acceptance limit. + int limit = JitConfig.JitInlineLimit(); + unsigned current = m_RootCompiler->m_inlineStrategy->GetInlineCount(); + + if (!m_IsPrejitRoot && (limit >= 0) && (current >= static_cast(limit))) + { + SetFailure(InlineObservation::CALLSITE_OVER_INLINE_LIMIT); + return; + } + +#endif // defined(DEBUG) + // Budget check. const bool overBudget = this->BudgetCheck(); if (overBudget) diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index cdd335e57c8f7e..535969f7963a00 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -137,6 +137,8 @@ CONFIG_INTEGER(JitReportFastTailCallDecisions, W("JitReportFastTailCallDecisions CONFIG_INTEGER(JitPInvokeCheckEnabled, W("JITPInvokeCheckEnabled"), 0) CONFIG_INTEGER(JitPInvokeEnabled, W("JITPInvokeEnabled"), 1) +CONFIG_INTEGER(JitHoistLimit, W("JitHoistLimit"), -1) // Specifies the maximum number of hoist candidates to hoist + // Controls verbosity for JitPrintInlinedMethods. Ignored for JitDump where // it's always set. CONFIG_INTEGER(JitPrintInlinedMethodsVerbose, W("JitPrintInlinedMethodsVerboseLevel"), 0) @@ -162,9 +164,7 @@ CONFIG_INTEGER(JitSsaStress, W("JitSsaStress"), 0) // Perturb order of processin CONFIG_INTEGER(JitStackChecks, W("JitStackChecks"), 0) CONFIG_INTEGER(JitStress, W("JitStress"), 0) // Internal Jit stress mode: 0 = no stress, 2 = all stress, other = vary // stress based on a hash of the method and this value -CONFIG_INTEGER(JitStressBBProf, W("JitStressBBProf"), 0) // Internal Jit stress mode -CONFIG_INTEGER(JitStressModeNamesOnly, W("JitStressModeNamesOnly"), 0) // Internal Jit stress: if nonzero, only enable - // stress modes listed in JitStressModeNames +CONFIG_INTEGER(JitStressBBProf, W("JitStressBBProf"), 0) // Internal Jit stress mode CONFIG_INTEGER(JitStressProcedureSplitting, W("JitStressProcedureSplitting"), 0) // Always split after the first basic // block. CONFIG_INTEGER(JitStressRegs, W("JitStressRegs"), 0) @@ -242,12 +242,24 @@ CONFIG_INTEGER(JitDumpFgBlockOrder, W("JitDumpFgBlockOrder"), 0) // 0 == bbNext CONFIG_INTEGER(JitDumpFgMemorySsa, W("JitDumpFgMemorySsa"), 0) // non-zero: show memory phis + SSA/VNs CONFIG_STRING(JitRange, W("JitRange")) -CONFIG_STRING(JitStressModeNames, W("JitStressModeNames")) // Internal Jit stress mode: stress using the given set of - // stress mode names, e.g. STRESS_REGS, STRESS_TAILCALL -CONFIG_STRING(JitStressModeNamesNot, W("JitStressModeNamesNot")) // Internal Jit stress mode: do NOT stress using the - // given set of stress mode names, e.g. STRESS_REGS, - // STRESS_TAILCALL -CONFIG_STRING(JitStressRange, W("JitStressRange")) // Internal Jit stress mode + +// Internal Jit stress mode: stress using the given set of stress mode names, e.g. STRESS_REGS, STRESS_TAILCALL. +// Unless JitStressModeNamesOnly is non-zero, other stress modes from a JitStress setting may also be invoked. +CONFIG_STRING(JitStressModeNames, W("JitStressModeNames")) + +// Internal Jit stress: if nonzero, only enable stress modes listed in JitStressModeNames. +CONFIG_INTEGER(JitStressModeNamesOnly, W("JitStressModeNamesOnly"), 0) + +// Internal Jit stress mode: only allow stress using the given set of stress mode names, e.g. STRESS_REGS, +// STRESS_TAILCALL. Note that JitStress must be enabled first, and then only the mentioned stress modes are allowed +// to be used, at the same percentage weighting as with JitStress -- the stress modes mentioned are NOT +// unconditionally true for a call to `compStressCompile`. This is basically the opposite of JitStressModeNamesNot. +CONFIG_STRING(JitStressModeNamesAllow, W("JitStressModeNamesAllow")) + +// Internal Jit stress mode: do NOT stress using the given set of stress mode names, e.g. STRESS_REGS, STRESS_TAILCALL +CONFIG_STRING(JitStressModeNamesNot, W("JitStressModeNamesNot")) + +CONFIG_STRING(JitStressRange, W("JitStressRange")) // Internal Jit stress mode CONFIG_METHODSET(JitEmitUnitTests, W("JitEmitUnitTests")) // Generate emitter unit tests in the specified functions CONFIG_STRING(JitEmitUnitTestsSections, W("JitEmitUnitTestsSections")) // Generate this set of unit tests @@ -255,7 +267,8 @@ CONFIG_STRING(JitEmitUnitTestsSections, W("JitEmitUnitTestsSections")) // Genera /// JIT Hardware Intrinsics /// CONFIG_INTEGER(EnableIncompleteISAClass, W("EnableIncompleteISAClass"), 0) // Enable testing not-yet-implemented -#endif // defined(DEBUG) + +#endif // defined(DEBUG) CONFIG_METHODSET(JitDisasm, W("JitDisasm")) // Print codegen for given methods CONFIG_INTEGER(JitDisasmTesting, W("JitDisasmTesting"), 0) // Display BEGIN METHOD/END METHOD anchors for disasm testing @@ -510,7 +523,9 @@ CONFIG_INTEGER(JitDoValueNumber, W("JitDoValueNumber"), 1) // Perform value numb CONFIG_INTEGER(JitEnableOptRepeat, W("JitEnableOptRepeat"), 1) // If zero, do not allow JitOptRepeat CONFIG_METHODSET(JitOptRepeat, W("JitOptRepeat")) // Runs optimizer multiple times on the method CONFIG_INTEGER(JitOptRepeatCount, W("JitOptRepeatCount"), 2) // Number of times to repeat opts when repeating -CONFIG_INTEGER(JitDoIfConversion, W("JitDoIfConversion"), 1) // Perform If conversion +CONFIG_STRING(JitOptRepeatRange, W("JitOptRepeatRange")) // Enable JitOptRepeat based on method hash range + +CONFIG_INTEGER(JitDoIfConversion, W("JitDoIfConversion"), 1) // Perform If conversion #endif // defined(OPT_CONFIG) diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 4e192a2567f480..25322a168a72a2 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -4596,6 +4596,15 @@ void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop, // hence this check is not present in optIsCSEcandidate(). return true; } + else if ((node->gtFlags & GTF_ORDER_SIDEEFF) != 0) + { + // If a node has an order side effect, we can't hoist it at all: we don't know what the order + // dependence actually is. For example, assertion prop might have determined a node can't throw + // an exception, and eliminated the GTF_EXCEPT flag, replacing it with GTF_ORDER_SIDEEFF. We + // can't hoist because we might then hoist above the expression that led assertion prop to make + // that decision. This can happen in JitOptRepeat, where hoisting can follow assertion prop. + return false; + } // Tree must be a suitable CSE candidate for us to be able to hoist it. return m_compiler->optIsCSEcandidate(node); @@ -4786,7 +4795,7 @@ void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop, } else if (!top.m_hoistable) { - top.m_failReason = "not handled by cse"; + top.m_failReason = "not handled by hoisting or CSE"; } #endif @@ -4869,7 +4878,7 @@ void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop, treeIsHoistable = IsNodeHoistable(tree); if (!treeIsHoistable) { - INDEBUG(failReason = "not handled by cse";) + INDEBUG(failReason = "not handled by hoisting or CSE";) } } @@ -5167,6 +5176,21 @@ void Compiler::optHoistCandidate(GenTree* tree, return; } +#if defined(DEBUG) + + // Punt if we've reached the hoisting limit. + int limit = JitConfig.JitHoistLimit(); + unsigned current = m_totalHoistedExpressions; // this doesn't include the current candidate yet + + if ((limit >= 0) && (current >= static_cast(limit))) + { + JITDUMP(" ... not hoisting in " FMT_LP ", hoist count %u >= JitHoistLimit %u\n", loop->GetIndex(), current, + static_cast(limit)); + return; + } + +#endif // defined(DEBUG) + // Expression can be hoisted optPerformHoistExpr(tree, treeBb, loop);