Stop using a mapping table between MethodDesc and InterpMethod

InterpMethod* is used to handle interpreted method execution on mono, but it is not really needed. For CoreCLR we will turn this structure in a header to the compiled IR code, and identify other methods directly by the IR code pointer. The first pointer slot in the IR code will contain a pointer to the InterpMethod containing any other relevant information needed in the method execution.

Author: BrzVlad

src/coreclr/inc/corjit.h

@@ -104,12 +104,6 @@ class ICorJitInfo;
 
 extern "C" ICorJitCompiler* getJit();
 
-class ICorInterpreter
-{
-    public:
-        virtual void* GetInterpMethod(CORINFO_METHOD_HANDLE methodHnd) = 0;
-};
-
 // #EEToJitInterface
 // ICorJitCompiler is the interface that the EE uses to get IL bytecode converted to native code. Note that
 // to accomplish this the JIT has to call back to the EE to get symbolic information.  The code:ICorJitInfo

src/coreclr/interpreter/clrinterpreter.exports

@@ -2,5 +2,4 @@
 ; The .NET Foundation licenses this file to you under the MIT license.
 EXPORTS
     getJit
-    getInterpreter
     jitStartup

src/coreclr/interpreter/clrinterpreter_unixexports.src

@@ -1,3 +1,2 @@
 getJit
-getInterpreter
 jitStartup

src/coreclr/interpreter/compiler.cpp

@@ -474,13 +474,21 @@ void InterpCompiler::EmitCode()
             ip = EmitCodeIns(ip, ins);
         }
     }
+
+    m_MethodCodeSize = (int32_t)(ip - m_pMethodCode);
 }
 
-void InterpCompiler::PublishInterpMethod(InterpMethod* pMethod)
+InterpMethod* InterpCompiler::CreateInterpMethod()
 {
-    pMethod->pCode = m_pMethodCode;
-    pMethod->allocaSize = m_totalVarsStackSize;
-    pMethod->compiled = true;
+    InterpMethod *pMethod = new InterpMethod(m_methodHnd, m_totalVarsStackSize);
+
+    return pMethod;
+}
+
+int32_t* InterpCompiler::GetCode(int32_t *pCodeSize)
+{
+    *pCodeSize = m_MethodCodeSize;
+    return m_pMethodCode;
 }
 
 InterpCompiler::InterpCompiler(COMP_HANDLE compHnd,
@@ -491,7 +499,7 @@ InterpCompiler::InterpCompiler(COMP_HANDLE compHnd,
     m_methodInfo = methodInfo;
 }
 
-int InterpCompiler::CompileMethod(InterpMethod *pMethod)
+InterpMethod* InterpCompiler::CompileMethod()
 {
     GenerateCode(m_methodInfo);
 
@@ -501,9 +509,7 @@ int InterpCompiler::CompileMethod(InterpMethod *pMethod)
 
     EmitCode();
 
-    PublishInterpMethod(pMethod);
-
-    return CORJIT_OK;
+    return CreateInterpMethod();
 }
 
 int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo)

src/coreclr/interpreter/compiler.h

@@ -206,18 +206,21 @@ class InterpCompiler
 
     // Passes
     int32_t* m_pMethodCode;
+    int32_t m_MethodCodeSize; // in int32_t
 
     void OptimizeCode();
     void AllocOffsets();
     int32_t ComputeCodeSize();
     void EmitCode();
     int32_t* EmitCodeIns(int32_t *ip, InterpInst *pIns);
-    void PublishInterpMethod(InterpMethod *pMethod);
+    InterpMethod* CreateInterpMethod();
 public:
 
     InterpCompiler(COMP_HANDLE compHnd, CORINFO_METHOD_INFO* methodInfo);
 
-    int CompileMethod(InterpMethod *pMethod);
+    InterpMethod* CompileMethod();
+
+    int32_t* GetCode(int32_t *pCodeSize);
 };
 
 #endif //_COMPILER_H_

src/coreclr/interpreter/eeinterp.cpp

@@ -15,30 +15,6 @@
 #define INTERP_API __attribute__ ((visibility ("default")))
 #endif // _MSC_VER
 
-#include <vector>
-
-// FIXME We will probably end up not needing this table.
-// If deemed useful, use some hashtable implementation instead.
-std::vector<std::pair<CORINFO_METHOD_HANDLE,InterpMethod*>> g_interpCodeHash;
-
-static InterpMethod* InterpGetInterpMethod(CORINFO_METHOD_HANDLE methodHnd)
-{
-    // FIXME lock for multiple thread access
-    for (size_t i = 0; i < g_interpCodeHash.size(); i++)
-    {
-        if (g_interpCodeHash[i].first == methodHnd)
-        {
-            return g_interpCodeHash[i].second;
-        }
-    }
-
-    InterpMethod* pMethod = new InterpMethod();
-    pMethod->methodHnd = methodHnd;
-
-    g_interpCodeHash.push_back({methodHnd, pMethod});
-    return pMethod;
-}
-
 /*****************************************************************************/
 ICorJitHost* g_interpHost        = nullptr;
 bool         g_interpInitialized = false;
@@ -64,16 +40,6 @@ extern "C" INTERP_API ICorJitCompiler* getJit()
     return &g_CILInterp;
 }
 
-static InterpManager g_Manager;
-extern "C" INTERP_API ICorInterpreter* getInterpreter()
-{
-    if (!g_interpInitialized)
-    {
-        return nullptr;
-    }
-    return &g_Manager;
-}
-
 //****************************************************************************
 CorJitResult CILInterp::compileMethod(ICorJitInfo*         compHnd,
                                    CORINFO_METHOD_INFO* methodInfo,
@@ -95,33 +61,36 @@ CorJitResult CILInterp::compileMethod(ICorJitInfo*         compHnd,
         return CORJIT_SKIPPED;
     }
 
-    InterpMethod *pMethod = InterpGetInterpMethod(methodInfo->ftn);
-    if (!pMethod->compiled)
-    {
-        InterpCompiler compiler(compHnd, methodInfo);
-        compiler.CompileMethod(pMethod);
-    }
+    InterpCompiler compiler(compHnd, methodInfo);
+    InterpMethod *pMethod = compiler.CompileMethod();
+
+    int32_t IRCodeSize;
+    int32_t *pIRCode = compiler.GetCode(&IRCodeSize);
 
     // FIXME this shouldn't be here
     compHnd->setMethodAttribs(methodInfo->ftn, CORINFO_FLG_INTERPRETER);
 
+    uint32_t sizeOfCode = sizeof(InterpMethod*) + IRCodeSize * sizeof(int32_t);
+
     // TODO: get rid of the need to allocate fake unwind info.
     compHnd->reserveUnwindInfo(false /* isFunclet */, false /* isColdCode */ , 8 /* unwindSize */);
     AllocMemArgs args;
-    args.hotCodeSize = 16;
+    args.hotCodeSize = sizeOfCode;
     args.coldCodeSize = 0;
     args.roDataSize = 0;
     args.xcptnsCount = 0;
     args.flag = CORJIT_ALLOCMEM_DEFAULT_CODE_ALIGN;
     compHnd->allocMem(&args);
-    uint8_t *code = (uint8_t*)args.hotCodeBlockRW;
-    *code++ = 1; // fake byte code
+
+    // We store first the InterpMethod pointer as the code header, followed by the actual code
+    *(InterpMethod**)args.hotCodeBlockRW = pMethod;
+    memcpy ((uint8_t*)args.hotCodeBlockRW + sizeof(InterpMethod*), pIRCode, IRCodeSize * sizeof(int32_t));
 
     // TODO: get rid of the need to allocate fake unwind info
     compHnd->allocUnwindInfo((uint8_t*)args.hotCodeBlock, (uint8_t*)args.coldCodeBlock, 0, 1, 0, nullptr, CORJIT_FUNC_ROOT);
 
     *entryAddress = (uint8_t*)args.hotCodeBlock;
-    *nativeSizeOfCode = 1;
+    *nativeSizeOfCode = sizeOfCode;
 
     return CORJIT_OK;
 }
@@ -140,8 +109,3 @@ void CILInterp::getVersionIdentifier(GUID* versionIdentifier)
 void CILInterp::setTargetOS(CORINFO_OS os)
 {
 }
-
-void* InterpManager::GetInterpMethod(CORINFO_METHOD_HANDLE methodHnd)
-{
-    return InterpGetInterpMethod(methodHnd);
-}

src/coreclr/interpreter/eeinterp.h

@@ -15,8 +15,3 @@ class CILInterp : public ICorJitCompiler
     void getVersionIdentifier(GUID* versionIdentifier /* OUT */ );
     void setTargetOS(CORINFO_OS os);
 };
-
-class InterpManager : public ICorInterpreter
-{
-    void* GetInterpMethod(CORINFO_METHOD_HANDLE methodHnd);
-};

src/coreclr/interpreter/interpretershared.h

@@ -20,15 +20,12 @@ typedef enum
 struct InterpMethod
 {
     CORINFO_METHOD_HANDLE methodHnd;
-    int32_t *pCode;
     int32_t allocaSize;
 
-    volatile bool compiled;
-
-    InterpMethod()
+    InterpMethod(CORINFO_METHOD_HANDLE methodHnd, int32_t allocaSize)
     {
-        pCode = NULL;
-        compiled = false;
+        this->methodHnd = methodHnd;
+        this->allocaSize = allocaSize;
     }
 };
 

src/coreclr/vm/codeman.cpp

@@ -1838,18 +1838,6 @@ static void LoadAndInitializeJIT(LPCWSTR pwzJitName DEBUGARG(LPCWSTR pwzJitPath)
     }
 }
 
-#ifdef FEATURE_INTERPRETER
-void InitializeInterpreter(IN HINSTANCE phJit, OUT ICorInterpreter **pNewInterpreter)
-{
-    typedef ICorInterpreter* (* pGetInterpreterFn)();
-    pGetInterpreterFn getInterpreterFn = (pGetInterpreterFn) GetProcAddress(phJit, "getInterpreter");
-    if (getInterpreterFn != NULL)
-    {
-        *pNewInterpreter = (*getInterpreterFn)();
-    }
-}
-#endif
-
 #ifdef FEATURE_MERGE_JIT_AND_ENGINE
 EXTERN_C void jitStartup(ICorJitHost* host);
 EXTERN_C ICorJitCompiler* getJit();
@@ -1911,9 +1899,6 @@ BOOL EEJitManager::LoadJIT()
     // This allows us to display load error messages for loading altjit.
 
     ICorJitCompiler* newAltJitCompiler = NULL;
-#ifdef FEATURE_INTERPRETER
-    ICorInterpreter* newInterpreter = NULL;
-#endif
 
     LPWSTR altJitConfig;
     IfFailThrow(CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_AltJit, &altJitConfig));
@@ -1985,9 +1970,6 @@ BOOL EEJitManager::LoadJIT()
         }
         g_JitLoadData.jld_id = JIT_LOAD_ALTJIT;
         LoadAndInitializeJIT(altJitName DEBUGARG(altJitPath), &m_AltJITCompiler, &newAltJitCompiler, &g_JitLoadData, targetOs);
-#ifdef FEATURE_INTERPRETER
-        InitializeInterpreter(m_AltJITCompiler, &newInterpreter);
-#endif
     }
 
 #endif // ALLOW_SXS_JIT
@@ -1997,9 +1979,6 @@ BOOL EEJitManager::LoadJIT()
 #ifdef ALLOW_SXS_JIT
     m_AltJITRequired = (altJitConfig != NULL);
     m_alternateJit = newAltJitCompiler;
-#ifdef FEATURE_INTERPRETER
-    m_interpreter = newInterpreter;
-#endif
 #endif // ALLOW_SXS_JIT
 
     m_jit = newJitCompiler;

src/coreclr/vm/codeman.h

@@ -1988,9 +1988,6 @@ private :
 #ifdef ALLOW_SXS_JIT
     //put these at the end so that we don't mess up the offsets in the DAC.
     ICorJitCompiler *   m_alternateJit;
-#ifdef FEATURE_INTERPRETER
-    ICorInterpreter *   m_interpreter;
-#endif
     HINSTANCE           m_AltJITCompiler;
     bool                m_AltJITRequired;
 #endif //ALLOW_SXS_JIT