-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix IL interpreter to run on Hello World #37825
Conversation
…r in the Debug/Checked configurations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the first runtime PR? Congrats :D
I made some suggestions inline and happy to chat anytime if you've got questions
If we want to allow arbitrary combinations of different code generators in the same process, the IsSupported values cannot vary between them. For example, somebody writes code like:
Consider what happens when the interpretation of the A similar problem exists with crossgen. We had bugs where IsSupported returned different values in crossgened core and JITed code, and it showed up as a real problem. I do not think we should be special casing the individual hardware intrinsics in the interpreter. As you have said, there is a lot of them. We should depend on fallback to JIT for the hardware intrinsics methods. The JIT does generate valid body for the hardware intrinsics methods that can be called e.g. via reflection.
I am fine with that - as long it is well documented that it is not a production quality solution. |
7cdd55f
to
ce301ad
Compare
… false for HW intrinsics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments inline. I think the name may need to be clarified a little for the INTERNAL_EnableJitOptimization
flag, but other than that, this is a great first PR!
Co-authored-by: John Salem <josalem@microsoft.com>
Given discussions with @noahfalk, @BruceForstall and @jkotas, I think that removing the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Happy interpretting : )
(Make sure to squash before merging)
Hi, But how to prove the Interpreter whether or not work?
Please have a look at debug.log Thanks, |
Hello @xiangzhai, Thanks for asking! Could you please share me with the Hello World sample program that you use for testing? Best, |
Hi @yuchong-pan Thanks for your response! I just Thanks, |
Hi @xiangzhai, Thanks for sharing me with this information! To see that the interpreter is actually used, one of the approaches would be to look at the information printed by I also tested this PR on your Hello World sample program, and it seems that the program is not interpreted because you set With Please let me know if you have further questions! Best, |
It's a little unfortunate that setting JITMinOpts disables the interpreter. What if you wanted to use the interpreter to bring up a new platform, where the JIT compiler is very immature? In that case, you might want to force JITMinOpts so the JIT does not invoke any optimization phases, but you would also want the interpreter as the "phase 0" of tiering. Maybe we need to rethink this MinOpts disabling tiering, in the presence of the interpreter. |
Hi @BruceForstall! I think the point you make is a very good scenario to think of. However, if we force |
It seems repro #13759
|
It is just able to work for MIPS64 #4234 (comment) by disabled optimization:
Yes, we are still fixing bugs to make it mature.
There are some hardcode disabled some optimization for MIPS64:
We are implementing it for MIPS64. Thanks, |
interpreter.cpp
to make the codebase compile on Windows whenFEATURE_INTERPRETER
is turned on.get_IsSupported
in the interpreter to semantically correctly interpret hardware intrinsics.COMPlus_
environment variablesForceInterpreterTier0
andForceInterpreterAlways
to force the interpreter to be used as Tier 0 and always, respectively. Note that these two environment variables respect others; for instance, ifInterpreterFallback
is set to1
, then JIT is still the default option. (Hence please check if the descriptions of the two environment variables are accurate.)Fixes #13759.
With the above changes to the codebase, the interpreter runs on the following Hello World sample program. The number of calls to
Not
is used to trigger the promotion to Tier 1 in Tiered Compilation.The following steps reproduce the result on the Hello World sample program (on Windows):
set(FEATURE_INTERPRETER 0)
insrc\coreclr\clrfeatures.cmake
..\build.cmd -subset clr.runtime
.COMPlus_
environment variables by the following (e.g. in Powershell):Current Priority 0 passing rate: