Fix coreclr interpreter build

[am11]

Tested on osx-arm64 with:

src/coreclr/build-runtime.sh -cmakeargs -DFEATURE_INTERPRETER=1

[am11]

@jkotas, I was trying to figure out what to replace the three HMF instances with in the interpreter:

runtime/src/coreclr/vm/interpreter.cpp

Lines 1880 to 1922 in b085dad

	HCIMPL3(float, InterpretMethodFloat, struct InterpreterMethodInfo* interpMethInfo, BYTE* ilArgs, void* stubContext)
	{
	FCALL_CONTRACT;
	ARG_SLOT retVal = 0;
	HELPER_METHOD_FRAME_BEGIN_RET_ATTRIB(Frame::FRAME_ATTR_EXACT_DEPTH|Frame::FRAME_ATTR_CAPTURE_DEPTH_2);
	retVal = (ARG_SLOT)Interpreter::InterpretMethodBody(interpMethInfo, false, ilArgs, stubContext);
	HELPER_METHOD_FRAME_END();
	return *reinterpret_cast<float*>(ArgSlotEndiannessFixup(&retVal, sizeof(float)));
	}
	HCIMPLEND
	// static
	HCIMPL3(double, InterpretMethodDouble, struct InterpreterMethodInfo* interpMethInfo, BYTE* ilArgs, void* stubContext)
	{
	FCALL_CONTRACT;
	ARG_SLOT retVal = 0;
	HELPER_METHOD_FRAME_BEGIN_RET_ATTRIB(Frame::FRAME_ATTR_EXACT_DEPTH|Frame::FRAME_ATTR_CAPTURE_DEPTH_2);
	retVal = Interpreter::InterpretMethodBody(interpMethInfo, false, ilArgs, stubContext);
	HELPER_METHOD_FRAME_END();
	return *reinterpret_cast<double*>(ArgSlotEndiannessFixup(&retVal, sizeof(double)));
	}
	HCIMPLEND
	// static
	HCIMPL3(INT64, InterpretMethod, struct InterpreterMethodInfo* interpMethInfo, BYTE* ilArgs, void* stubContext)
	{
	FCALL_CONTRACT;
	ARG_SLOT retVal = 0;
	HELPER_METHOD_FRAME_BEGIN_RET_ATTRIB(Frame::FRAME_ATTR_EXACT_DEPTH|Frame::FRAME_ATTR_CAPTURE_DEPTH_2);
	retVal = Interpreter::InterpretMethodBody(interpMethInfo, false, ilArgs, stubContext);
	HELPER_METHOD_FRAME_END();
	return static_cast<INT64>(retVal);
	}
	HCIMPLEND

and realized that it wasn't building.

[jkotas]

https://github.com/dotnet/runtime/blob/main/src/coreclr/vm/interpreter.cpp is throw-away code with many issues. We may want to delete it.

[AaronRobinsonMSFT]

I would prefer to leave interpreter "clean up" to the upcoming year. Piecemeal removal isn't moving the needle when all of this will be removed in due time. It also causes some issues with trying to understand the current state, which anyone looking at the interpreter might want to understand/internalize.

src/coreclr/build-runtime.sh -cmakeargs -DFEATURE_INTERPRETER=1

We have no scenario where we build with this and as such everything under it should be considered suspect.

My preference is to close this PR.

[janvorli]

I agree with @AaronRobinsonMSFT

[AaronRobinsonMSFT]

@am11 As @jkotas mentioned these three functions are "dead code" and will be removed in due time. See https://github.com/dotnet/runtimelab/tree/feature/CoreclrInterpreter for some experiments that were being run for a future release.

As I've been working through HMF removal, I've been ignoring the interpreter. You can too.

[am11]

@AaronRobinsonMSFT this PR is fixing interpreter build. It is unrelated to HMF. This is not the first PR fixing the interpreter build.

[jkotas]

Does something interesting work with these fixes? We got better insight into what we should do about it in last several months. This interpreter has number of fundamental design issues that are impossible to fix without complete rewrite.

@AaronRobinsonMSFT @janvorli Would you be fine with deleting FEATURE_INTERPRETER and all code under it? I do not think there is much to preserve.

[janvorli]

I would prefer that.

[AaronRobinsonMSFT]

@AaronRobinsonMSFT @janvorli Would you be fine with deleting FEATURE_INTERPRETER and all code under it? I do not think there is much to preserve.

I don't think anything under FEATURE_INTERPRETER should be touched or altered. I agree there isn't much to preserve from a "well tread path". I will say that removing it now will simply leave a myriad of dead ends and non-intuitive or ugly code paths. I'd prefer to let the team that will be looking at the interpreter in CoreCLR to see where things are and remove as needed. They can make decisions about how to plug-in in the same way or close them off cleanly and move forward.

I still think this PR should be closed and left for the future interpreter v-team.

[AaronRobinsonMSFT]

I would prefer that.

@janvorli Is likely to be on the interpreter v-team so if that makes sense to him, sounds good.

[am11]

Does something interesting work with these fixes?

Aside from simple hello world, I haven't tested much. This is a build fix, same as we did before in #34075 and similar from others. eng/pipelines/coreclr/clrinterpreter.yml has a leg which is disabled, so these breakages don't get caught in time.

I still think this PR should be closed and left for the future interpreter v-team.

This code is broken for months (if not longer). This tiny PR is fixing the build not the other way around?

[am11]

Superseded by #111137.