From a8d8b4a0f0b0ab3e14ba0e910cf5cb607584af03 Mon Sep 17 00:00:00 2001 From: Carol Eidt Date: Fri, 19 Jun 2020 09:21:27 -0700 Subject: [PATCH] JitStress Fixes (#38103) Initialize method info early so that jitStress can use that information to determine when to enable stress. Fix #8889 --- src/coreclr/src/jit/compiler.cpp | 109 ++++++++++++++++++++++++--------------- src/coreclr/src/jit/compiler.h | 11 ++-- src/coreclr/src/jit/lclvars.cpp | 3 -- 3 files changed, 74 insertions(+), 49 deletions(-) diff --git a/src/coreclr/src/jit/compiler.cpp b/src/coreclr/src/jit/compiler.cpp index f94b5fb..7591c7a 100644 --- a/src/coreclr/src/jit/compiler.cpp +++ b/src/coreclr/src/jit/compiler.cpp @@ -1719,23 +1719,62 @@ void Compiler::compDisplayStaticSizes(FILE* fout) * * Constructor */ - -void Compiler::compInit(ArenaAllocator* pAlloc, InlineInfo* inlineInfo) +void Compiler::compInit(ArenaAllocator* pAlloc, + CORINFO_METHOD_HANDLE methodHnd, + COMP_HANDLE compHnd, + CORINFO_METHOD_INFO* methodInfo, + InlineInfo* inlineInfo) { assert(pAlloc); compArenaAllocator = pAlloc; + // Inlinee Compile object will only be allocated when needed for the 1st time. + InlineeCompiler = nullptr; + + // Set the inline info. + impInlineInfo = inlineInfo; + info.compCompHnd = compHnd; + info.compMethodHnd = methodHnd; + info.compMethodInfo = methodInfo; + +#ifdef DEBUG + bRangeAllowStress = false; +#endif + #if defined(DEBUG) || defined(LATE_DISASM) + // Initialize the method name and related info, as it is used early in determining whether to + // apply stress modes, and which ones to apply. + // Note that even allocating memory can invoke the stress mechanism, so ensure that both + // 'compMethodName' and 'compFullName' are either null or valid before we allocate. + // (The stress mode checks references these prior to checking bRangeAllowStress.) + // info.compMethodName = nullptr; info.compClassName = nullptr; info.compFullName = nullptr; -#endif // defined(DEBUG) || defined(LATE_DISASM) - // Inlinee Compile object will only be allocated when needed for the 1st time. - InlineeCompiler = nullptr; + const char* classNamePtr; + const char* methodName; - // Set the inline info. - impInlineInfo = inlineInfo; + methodName = eeGetMethodName(methodHnd, &classNamePtr); + unsigned len = (unsigned)roundUp(strlen(classNamePtr) + 1); + info.compClassName = getAllocator(CMK_DebugOnly).allocate(len); + info.compMethodName = methodName; + strcpy_s((char*)info.compClassName, len, classNamePtr); + + info.compFullName = eeGetMethodFullName(methodHnd); + info.compPerfScore = 0.0; +#endif // defined(DEBUG) || defined(LATE_DISASM) + +#ifdef DEBUG + // Opt-in to jit stress based on method hash ranges. + // + // Note the default (with JitStressRange not set) is that all + // methods will be subject to stress. + static ConfigMethodRange fJitStressRange; + fJitStressRange.EnsureInit(JitConfig.JitStressRange()); + assert(!fJitStressRange.Error()); + bRangeAllowStress = fJitStressRange.Contains(info.compMethodHash()); +#endif // DEBUG eeInfoInitialized = false; @@ -1771,10 +1810,6 @@ void Compiler::compInit(ArenaAllocator* pAlloc, InlineInfo* inlineInfo) compJitTelemetry.Initialize(this); #endif -#ifdef DEBUG - bRangeAllowStress = false; -#endif - fgInit(); lvaInit(); @@ -2682,6 +2717,9 @@ void Compiler::compInitOptions(JitFlags* jitFlags) setUsesSIMDTypes(false); #endif // FEATURE_SIMD + lvaEnregEHVars = (((opts.compFlags & CLFLG_REGVAR) != 0) && JitConfig.EnableEHWriteThru()); + lvaEnregMultiRegVars = (((opts.compFlags & CLFLG_REGVAR) != 0) && JitConfig.EnableMultiRegLocals()); + if (compIsForImportOnly()) { return; @@ -5209,14 +5247,16 @@ bool Compiler::skipMethod() /*****************************************************************************/ -int Compiler::compCompile(CORINFO_METHOD_HANDLE methodHnd, - CORINFO_MODULE_HANDLE classPtr, - COMP_HANDLE compHnd, - CORINFO_METHOD_INFO* methodInfo, +int Compiler::compCompile(CORINFO_MODULE_HANDLE classPtr, void** methodCodePtr, ULONG* methodCodeSize, JitFlags* compileFlags) { + // compInit should have set these already. + noway_assert(info.compMethodInfo != nullptr); + noway_assert(info.compCompHnd != nullptr); + noway_assert(info.compMethodHnd != nullptr); + #ifdef FEATURE_JIT_METHOD_PERF static bool checkedForJitTimeLog = false; @@ -5227,7 +5267,7 @@ int Compiler::compCompile(CORINFO_METHOD_HANDLE methodHnd, // Call into VM to get the config strings. FEATURE_JIT_METHOD_PERF is enabled for // retail builds. Do not call the regular Config helper here as it would pull // in a copy of the config parser into the clrjit.dll. - InterlockedCompareExchangeT(&Compiler::compJitTimeLogFilename, compHnd->getJitTimeLogFilename(), NULL); + InterlockedCompareExchangeT(&Compiler::compJitTimeLogFilename, info.compCompHnd->getJitTimeLogFilename(), NULL); // At a process or module boundary clear the file and start afresh. JitTimer::PrintCsvHeader(); @@ -5236,7 +5276,7 @@ int Compiler::compCompile(CORINFO_METHOD_HANDLE methodHnd, } if ((Compiler::compJitTimeLogFilename != nullptr) || (JitTimeLogCsv() != nullptr)) { - pCompJitTimer = JitTimer::Create(this, methodInfo->ILCodeSize); + pCompJitTimer = JitTimer::Create(this, info.compMethodInfo->ILCodeSize); } #endif // FEATURE_JIT_METHOD_PERF @@ -5274,10 +5314,6 @@ int Compiler::compCompile(CORINFO_METHOD_HANDLE methodHnd, // if (s_compMethodsCount==0) setvbuf(jitstdout, NULL, _IONBF, 0); - info.compCompHnd = compHnd; - info.compMethodHnd = methodHnd; - info.compMethodInfo = methodInfo; - if (compIsForInlining()) { compileFlags->Clear(JitFlags::JIT_FLAG_OSR); @@ -5346,7 +5382,7 @@ int Compiler::compCompile(CORINFO_METHOD_HANDLE methodHnd, { impTokenLookupContextHandle = impInlineInfo->tokenLookupContextHandle; - assert(impInlineInfo->inlineCandidateInfo->clsHandle == compHnd->getMethodClass(methodHnd)); + assert(impInlineInfo->inlineCandidateInfo->clsHandle == info.compCompHnd->getMethodClass(info.compMethodHnd)); info.compClassHnd = impInlineInfo->inlineCandidateInfo->clsHandle; assert(impInlineInfo->inlineCandidateInfo->clsAttr == info.compCompHnd->getClassAttribs(info.compClassHnd)); @@ -5358,7 +5394,7 @@ int Compiler::compCompile(CORINFO_METHOD_HANDLE methodHnd, { impTokenLookupContextHandle = MAKE_METHODCONTEXT(info.compMethodHnd); - info.compClassHnd = compHnd->getMethodClass(methodHnd); + info.compClassHnd = info.compCompHnd->getMethodClass(info.compMethodHnd); info.compClassAttr = info.compCompHnd->getClassAttribs(info.compClassHnd); } @@ -5367,12 +5403,12 @@ int Compiler::compCompile(CORINFO_METHOD_HANDLE methodHnd, #if defined(DEBUG) || defined(LATE_DISASM) const char* classNamePtr; - info.compMethodName = eeGetMethodName(methodHnd, &classNamePtr); + info.compMethodName = eeGetMethodName(info.compMethodHnd, &classNamePtr); unsigned len = (unsigned)roundUp(strlen(classNamePtr) + 1); info.compClassName = getAllocator(CMK_DebugOnly).allocate(len); strcpy_s((char*)info.compClassName, len, classNamePtr); - info.compFullName = eeGetMethodFullName(methodHnd); + info.compFullName = eeGetMethodFullName(info.compMethodHnd); info.compPerfScore = 0.0; #endif // defined(DEBUG) || defined(LATE_DISASM) @@ -5392,15 +5428,6 @@ int Compiler::compCompile(CORINFO_METHOD_HANDLE methodHnd, return CORJIT_SKIPPED; } - // Opt-in to jit stress based on method hash ranges. - // - // Note the default (with JitStressRange not set) is that all - // methods will be subject to stress. - static ConfigMethodRange fJitStressRange; - fJitStressRange.EnsureInit(JitConfig.JitStressRange()); - assert(!fJitStressRange.Error()); - bRangeAllowStress = fJitStressRange.Contains(info.compMethodHash()); - #endif // DEBUG // Set this before the first 'BADCODE' @@ -5427,14 +5454,14 @@ int Compiler::compCompile(CORINFO_METHOD_HANDLE methodHnd, } param; param.pThis = this; param.classPtr = classPtr; - param.compHnd = compHnd; - param.methodInfo = methodInfo; + param.compHnd = info.compCompHnd; + param.methodInfo = info.compMethodInfo; param.methodCodePtr = methodCodePtr; param.methodCodeSize = methodCodeSize; param.compileFlags = compileFlags; param.result = CORJIT_INTERNALERROR; - setErrorTrap(compHnd, Param*, pParam, ¶m) // ERROR TRAP: Start normal block + setErrorTrap(info.compCompHnd, Param*, pParam, ¶m) // ERROR TRAP: Start normal block { pParam->result = pParam->pThis->compCompileHelper(pParam->classPtr, pParam->compHnd, pParam->methodInfo, @@ -6732,16 +6759,16 @@ START: assert(pParam->pComp != nullptr); #endif - pParam->pComp->compInit(pParam->pAlloc, pParam->inlineInfo); + pParam->pComp->compInit(pParam->pAlloc, pParam->methodHnd, pParam->compHnd, pParam->methodInfo, + pParam->inlineInfo); #ifdef DEBUG pParam->pComp->jitFallbackCompile = pParam->jitFallbackCompile; #endif // Now generate the code - pParam->result = - pParam->pComp->compCompile(pParam->methodHnd, pParam->classPtr, pParam->compHnd, pParam->methodInfo, - pParam->methodCodePtr, pParam->methodCodeSize, pParam->compileFlags); + pParam->result = pParam->pComp->compCompile(pParam->classPtr, pParam->methodCodePtr, pParam->methodCodeSize, + pParam->compileFlags); } finallyErrorTrap() { diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h index 42111f6..68bbc56 100644 --- a/src/coreclr/src/jit/compiler.h +++ b/src/coreclr/src/jit/compiler.h @@ -9321,7 +9321,11 @@ public: static void compStartup(); // One-time initialization static void compShutdown(); // One-time finalization - void compInit(ArenaAllocator* pAlloc, InlineInfo* inlineInfo); + void compInit(ArenaAllocator* pAlloc, + CORINFO_METHOD_HANDLE methodHnd, + COMP_HANDLE compHnd, + CORINFO_METHOD_INFO* methodInfo, + InlineInfo* inlineInfo); void compDone(); static void compDisplayStaticSizes(FILE* fout); @@ -9344,10 +9348,7 @@ public: void compDoComponentUnitTestsOnce(); #endif // DEBUG - int compCompile(CORINFO_METHOD_HANDLE methodHnd, - CORINFO_MODULE_HANDLE classPtr, - COMP_HANDLE compHnd, - CORINFO_METHOD_INFO* methodInfo, + int compCompile(CORINFO_MODULE_HANDLE classPtr, void** methodCodePtr, ULONG* methodCodeSize, JitFlags* compileFlags); diff --git a/src/coreclr/src/jit/lclvars.cpp b/src/coreclr/src/jit/lclvars.cpp index b2c95af..af8e301 100644 --- a/src/coreclr/src/jit/lclvars.cpp +++ b/src/coreclr/src/jit/lclvars.cpp @@ -86,9 +86,6 @@ void Compiler::lvaInit() lvaCurEpoch = 0; structPromotionHelper = new (this, CMK_Generic) StructPromotionHelper(this); - - lvaEnregEHVars = (((opts.compFlags & CLFLG_REGVAR) != 0) && JitConfig.EnableEHWriteThru()); - lvaEnregMultiRegVars = (((opts.compFlags & CLFLG_REGVAR) != 0) && JitConfig.EnableMultiRegLocals()); } /*****************************************************************************/ -- 2.7.4