JitStress Fixes (#38103)
authorCarol Eidt <carol.eidt@microsoft.com>
Fri, 19 Jun 2020 16:21:27 +0000 (09:21 -0700)
committerGitHub <noreply@github.com>
Fri, 19 Jun 2020 16:21:27 +0000 (09:21 -0700)
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
src/coreclr/src/jit/compiler.h
src/coreclr/src/jit/lclvars.cpp

index f94b5fb..7591c7a 100644 (file)
@@ -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<char>(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<char>(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, &param) // ERROR TRAP: Start normal block
+    setErrorTrap(info.compCompHnd, Param*, pParam, &param) // 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()
         {
index 42111f6..68bbc56 100644 (file)
@@ -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);
index b2c95af..af8e301 100644 (file)
@@ -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());
 }
 
 /*****************************************************************************/