CoreCLR interpreter basic compilation and execution #112369

BrzVlad · 2025-02-10T21:43:59Z

In this stage, the implementation does not account for compilation races between multiple threads. Some of the code/functionality will be used in further commits. Additionally there are placeholders for memory allocation, that just use malloc. In the future, most of the memory used during compilation should be cheaply allocated from a compilation memory pool. Some other compilation related memory might need to be freed early to prevent memory waste for huge methods. It would actually make sense for this memory to be allocated with malloc. Other data will need to have its lifetime tied to the MethodDesc lifetime, so if a MethodDesc is ever released, a callback into the interpreter would be necessary to free up this data. These problems are not really relevant in the early stage of the prototype and they can be cleanly addressed later on in the implementation.

The PR is split up in a few commits that can be reviewed independently for convenience.

dotnet-policy-service · 2025-02-10T22:29:27Z

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

src/coreclr/interpreter/ee_il_dll.cpp

src/coreclr/inc/corjit.h

src/coreclr/interpreter/compiler.h

src/coreclr/interpreter/ee_il_dll.cpp

kg · 2025-02-11T14:30:01Z

src/coreclr/interpreter/ee_il_dll.cpp

+/*****************************************************************************/
+extern "C" DLLEXPORT void jitStartup(ICorJitHost* jitHost)
+{
+    if (g_interpInitialized)


should we check and throw if someone calls startup again with a different jit host?

kg · 2025-02-11T14:32:10Z

src/coreclr/interpreter/ee_il_dll.cpp

+    InterpMethod *pMethod = InterpGetInterpMethod(methodInfo->ftn);
+    pMethod->compiled = true;
+
+    compHnd->setMethodAttribs(methodInfo->ftn, CORINFO_FLG_INTERPRETER);


Is this a race in the future if we set the INTERPRETER flag before we're done initializing all the interpreter state and metadata for the method? Or do we actually want to set it early so that once threading is wired up, other threads will see the flag and wait for us to finish compiling?

This flag should not be set by the interpreter JIT in the first place. The synchronization for multiple threads trying to JIT a method at the same and other global state management should be done on the VM side around the call to the interpreter JIT, like it is done for regular JIT.

Added a FIXME for now. Seems to me that this is something best refactored once we stop relying on the AltJit for kicking interpreter compilation.

src/coreclr/interpreter/compiler.cpp

kg · 2025-02-11T21:07:10Z

One general note from reviewing is that we probably need handling for allocation failure in the interpreter. Since it has its own helpers for allocation it should be straightforward to centralize it and i.e. do a null check and throw/assert on allocation failure inside the helper so that the interp doesn't have to check for null every time it allocates.

On most targets attempting to deref a NULL allocation would fail fast but on wasm the interpreter would stomp all over memory, which isn't great, so we do need the explicit checks.

kg · 2025-02-11T21:15:59Z

LGTM overall so far

src/coreclr/interpreter/compiler.h

src/coreclr/inc/corjit.h

src/coreclr/interpreter/eeinterp.cpp

janvorli · 2025-02-17T13:55:17Z

src/coreclr/interpreter/eeinterp.cpp

+    int32_t *pIRCode = compiler.GetCode(&IRCodeSize);
+
+    // FIXME this shouldn't be here
+    compHnd->setMethodAttribs(methodInfo->ftn, CORINFO_FLG_INTERPRETER);


@jkotas these are special attributes that are used for communicating details back to the runtime. The JIT does the same thing. See e.g. the CORINFO_FLG_SWITCHED_TO_OPTIMIZED usage. These flags are special and are not set on the MethodDesc. I see reporting back that the JIT generated interpreted code using this flag as something in line with how the other JIT flags are used.

These special attributes are used for communicating details that that runtime cannot possibly know. They are computed from non-trivial analysis of the IL. This detail is not like that.

The runtime should know upfront that it is about to call JIT to generate interpreter code. This is related to our discussion about the dedicated code manager for interpreter code in the other PR.

Also, there is getExpectedTargetArchitecture JIT/EE interface API. This API should return a new unique architecture value that identifies interpreter. It requires runtime to know that we are generating interpreter code upfront as well.

I was hoping that during the bringup, we would use the AltJIT mechanism. So the interpreter itself decides which methods it can compile and the runtime falls back to regular JIT for the ones that it rejects. At some later point in the bringup, it could even fail to compile some methods in the middle due to encountering IL opcode that it doesn't support yet. I think it would be quite helpful during the bringup process. So the runtime doesn't know upfront whether the JIT or the interpreter is going to be used. That's why we are using a mechanism that allows us to get the information on the generated code type post invocation of the JIT.
Once the interpreter is fully functional, we will stop using the AltJIT mechanism and on the platforms where we cannot JIT, it would replace the JIT. If we'd find use cases for the interpreter side by side with JIT, then the decision whether to call the JIT or the interpreter will be made in the runtime. So in the final state, I don't think we will need this method attribute anymore.

So the interpreter itself decides which methods it can compile and the runtime falls back to regular JIT for the ones that it rejects

CORJIT_SKIPPED return code (or any other failure code) from the JIT is meant for this purpose. The current altjit mechanism uses it to communicate that the aljit was not interested in compiling the method and regular JIT should be used as a fallback.

Yes, that's the way it works with the interpreter too. The reason I was mentioning this is that the runtime doesn't know if the AltJIT is another JIT or the interpreter. So the new attribute is a way to communicate that back to the caller that needs to know that without introducing something else for the runtime to recognize that the AltJIT produced interpreter IR byte code. We could come up with possibly many other ways to do that (setting a bit on the code address returned by the interpreter, adding a new API to the JIT to get such info etc.) However, the attribute seemed to be the least intrusive way to achieve that.

It is ok for now, but I am not convinced that it is the design we want in the long run.

src/coreclr/interpreter/compiler.cpp

src/coreclr/interpreter/eeinterp.h

src/coreclr/vm/interpexec.cpp

src/coreclr/interpreter/eeinterp.cpp

src/coreclr/vm/interpexec.h

janvorli

LGTM, thank you!

src/coreclr/interpreter/compiler.cpp

BrzVlad · 2025-02-20T14:06:03Z

@jkotas Is this good to merge ?

jkotas

@jkotas Is this good to merge ?

Fine with me. I have not reviewed the changes in detail.

(left a minor comment)

jkotas · 2025-02-20T18:05:31Z

src/coreclr/interpreter/compiler.h

+    int32_t* m_pMethodCode;
+    int32_t m_MethodCodeSize; // in int32_t
+
+    void OptimizeCode();


Suggested change

void OptimizeCode();

jkotas · 2025-02-20T18:07:09Z

src/coreclr/interpreter/compileropt.cpp

+
+void InterpCompiler::OptimizeCode()
+{
+    // TODO
+}


Suggested change

void InterpCompiler::OptimizeCode()

{

// TODO

}

We should not be investing into code optimizations in this interpreter. Optimizing interpreter should be built from RyuJIT codebase and leverage all optimizations that RyuJIT does.

Optimization in general for the interpreter is something far away, at least 1 year, so no work will be done on it for the prototype anyway. I think reusing RyuJIT optimizations requires a complete redesign of the interpreter code gen so it has a similar way of representing all the compilation data. Doesn't seem very attractive to me, especially given we already have a set of decently advanced optimization on mono that can plug in very easily. So it seems like much work for little gain at this moment.

Doesn't seem very attractive to me, especially given we already have a set of decently advanced optimization on mono that can plug in very easily.

The primary goal of this project is to minimize our long-term costs. We do not want to be maintaining two advanced optimizers.

I think reusing RyuJIT optimizations requires a complete redesign of the interpreter code gen so it has a similar way of representing all the compilation data.

Once we get there, we should look at this as building RyuJIT backend that produces interpreter byte code.

This commit adds a new library for the interpreter. This library is meant to be used as an alt jit compiler, exporting the `getJit` method that returns an interface for method compilation. This library also exports the `getInterpreter` method that returns a separate interface to be used for obtaining any other data from the interpreter library. Method compilation has the following responsability: - create an InterpMethod* for the MethodDesc that it flags as compiled. On mono, InterpMethod serves as reference to the method to be called (which is embedded throughout the code) and later on, following compilation, it is filled with additional information relevant for execution, mainly the code pointer and size of the frame. On CoreCLR, this structure will likely lose some of its relevance, we will end up referencing MethodDesc's directly in interpreter instructions. In the future, it will probably make more sense to have a few common fields of the InterpMethod (like the frame size) stored as a code header to the interpreter IR, with a InterpMethod pointer to be stored as well in the code header for less frequent data. - set the CORINFO_FLG_INTERPRETER flag in the MethodDesc. This sets up the method to be called through the InterpreterStub. - some dummy data that is necessary by the runtime is being generated, mainly the code The interpreter execution is included in the main coreclr library for convenience and speed. It will be contained in interpexec.h and interpexec.cpp while also including the interpretershared.h header from the interpreter library. This header will contain any defines and types that are relevant both during compilation as well as execution (for example the InterpMethod data or the opcode values). For the execution, InterpFrame is a structure on the native stack that is created for each interpreter frame. It will reference the method being executed as well as pointers to the interpreter stack. InterpThreadContext is a thread local structure that contains execution relevant information for the interpreter, for now it just maintains the position of the interpreter stack pointer.

This commit ports over some of the compilation machinery from mono. Method compilation takes place in GenerateCode. It allocates a basic block, goes over the IL code and emits code as it goes, while updating the IL stack state. Method code is kept as a linked list of basic blocks. Each basic block keeps a linked list of instructions. The instruction information is represented by an opcode, source and destination vars and additional instruction data. Once the IL code is imported, the var offset allocator needs to kick in in order to allocate an offset for each variable referenced in the code. Once offsets are allocated the final interpreter IR can be generated, writing the opcode and instruction data linearly in the code, while replacing variable indexes with the actual variable offset. Unlike the mono interpreter, that maintains the code as an uint16_t stream, we chose to use from the start an int32_t based stream. The mono interpreter runs into limitations when executing very large methods, where some offsets don't fit in the uint16_t space. There will be a performance penalty for this that will be addressed in the distant future, by adding short versions for common opcodes.

The test contains one application that contains a method named `RunInterpreterTests` serving as the interpreter entry point. This application is built only, as it is not the test itself. This application is executed with corerun by the actual test, which waits for the corerun exit code to determine success.

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.

dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 10, 2025

dotnet-policy-service bot assigned BrzVlad Feb 10, 2025

jkotas requested a review from janvorli February 10, 2025 22:28

jkotas added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Feb 10, 2025

jkotas reviewed Feb 10, 2025

View reviewed changes

src/coreclr/interpreter/ee_il_dll.cpp Outdated Show resolved Hide resolved

This was referenced Feb 11, 2025

slow macOS - "##[error]The job running on agent Azure Pipelines 9 ran longer than the maximum time of 60 minutes." dotnet/dnceng#1883

Open

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

steveisok requested review from kg and lateralusX February 11, 2025 12:51