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

Build the legacy jit for x86. #2214

Merged
merged 1 commit into from Dec 15, 2015

Conversation

Projects
None yet
8 participants
@jashook
Contributor

jashook commented Dec 2, 2015

Small changes to cmake, which allows us to build the legacy jit as the fallback jit for x86.

**** NOTE ****

This change will break the x86 build. It is not ready to merge until the legacy jit link errors are fixed. However, I would like to put it out for review.

@BruceForstall PTAL

Update: created a new cmakelists file to build a seperate ryujit x86 which can be used as the alt jit with the legacy jit linked statically into coreclr.

@benpye

This comment has been minimized.

Show comment
Hide comment
@benpye

benpye Dec 3, 2015

Contributor

See @BruceForstall 's comment in #1210 . Has that situation changed?

Contributor

benpye commented Dec 3, 2015

See @BruceForstall 's comment in #1210 . Has that situation changed?

@jashook

This comment has been minimized.

Show comment
Hide comment
@jashook

jashook Dec 3, 2015

Contributor

I will give a quick answer. Bruce and others can add to it, or correct it as they see fit.

His comment has not changed, the legacy jit is not supported and to my knowledge will not be. Instead, the value it has for us is the ability to use it as a fallback jit to bring up x86.

This change allows the legacy jit to act as the default jit and the separate RyuJit dll to be an altjit.

Contributor

jashook commented Dec 3, 2015

I will give a quick answer. Bruce and others can add to it, or correct it as they see fit.

His comment has not changed, the legacy jit is not supported and to my knowledge will not be. Instead, the value it has for us is the ability to use it as a fallback jit to bring up x86.

This change allows the legacy jit to act as the default jit and the separate RyuJit dll to be an altjit.

@jkotas

View changes

Show outdated Hide outdated src/jit/standalone/CMakeLists.txt
add_definitions(-DFEATURE_REF_ZERO_OFFSET_ALLOWED)
# We will need to build ryujit as an altjit for targets we
# will be bringing up the jit in.

This comment has been minimized.

@jkotas

jkotas Dec 3, 2015

Member

I do not understand how this comment relates to the ifdef you have added

@jkotas

jkotas Dec 3, 2015

Member

I do not understand how this comment relates to the ifdef you have added

This comment has been minimized.

@jkotas

jkotas Dec 3, 2015

Member

The ryujit .dll packaged into RyuJIT nuget package should always be the proper RyuJIT, otherwise it will be source of endless confusion. If you would like to do mixing and matching for CoreRT, it should be done by forwarding JIT .dll that implements the multiplexing policy. @russellhadley was talking about having one for mixing and matching ryujit and llilc.

@jkotas

jkotas Dec 3, 2015

Member

The ryujit .dll packaged into RyuJIT nuget package should always be the proper RyuJIT, otherwise it will be source of endless confusion. If you would like to do mixing and matching for CoreRT, it should be done by forwarding JIT .dll that implements the multiplexing policy. @russellhadley was talking about having one for mixing and matching ryujit and llilc.

@jkotas

This comment has been minimized.

Show comment
Hide comment
@jkotas

jkotas Dec 3, 2015

Member

the legacy jit is not supported and to my knowledge will not be

There is nuance in the definition of "supported": it means that Microsoft does not plan to ship it in a officially supported product, but it will likely work reasonably well for foreseeable future - because of it will be used as fallback path for bring up of the proper ryujit backends.

cc @cmckinsey

Member

jkotas commented Dec 3, 2015

the legacy jit is not supported and to my knowledge will not be

There is nuance in the definition of "supported": it means that Microsoft does not plan to ship it in a officially supported product, but it will likely work reasonably well for foreseeable future - because of it will be used as fallback path for bring up of the proper ryujit backends.

cc @cmckinsey

@cmckinsey

This comment has been minimized.

Show comment
Hide comment
@cmckinsey

cmckinsey Dec 4, 2015

Contributor

@benpye We intend to keep the LEGACY_BACKEND x86 and ARM32 code-generation paths online in the code base and at the current functionality level until such time when can replace them with a roughly equivalent code-generator based on the new RyuJIT path. For x64 and ARM64 development we will only accept changes that extend the new RyuJIT backend path.

Here's my guidance on how non-MS contributors should think about contributing to the JIT: If you want to help advance the state of the production code-generators for .NET, then contribute to the new RyuJIT x86/ARM32 backend. This is our long term direction. If instead your interest is around getting the .NET Core runtime working on x86 or ARM32 platforms to do other things, by all means use and contribute bug fixes if necessary to the LEGACY_BACKEND paths in the RyuJIT code base today to unblock yourself. We do run testing on these paths today in our internal testing infrastructure and will do our best to avoid regressing it until we can replace it with something better. We just want to make sure that there will be no surprises or hard feelings for when the time comes to remove them from the code-base.

Contributor

cmckinsey commented Dec 4, 2015

@benpye We intend to keep the LEGACY_BACKEND x86 and ARM32 code-generation paths online in the code base and at the current functionality level until such time when can replace them with a roughly equivalent code-generator based on the new RyuJIT path. For x64 and ARM64 development we will only accept changes that extend the new RyuJIT backend path.

Here's my guidance on how non-MS contributors should think about contributing to the JIT: If you want to help advance the state of the production code-generators for .NET, then contribute to the new RyuJIT x86/ARM32 backend. This is our long term direction. If instead your interest is around getting the .NET Core runtime working on x86 or ARM32 platforms to do other things, by all means use and contribute bug fixes if necessary to the LEGACY_BACKEND paths in the RyuJIT code base today to unblock yourself. We do run testing on these paths today in our internal testing infrastructure and will do our best to avoid regressing it until we can replace it with something better. We just want to make sure that there will be no surprises or hard feelings for when the time comes to remove them from the code-base.

@richlander

This comment has been minimized.

Show comment
Hide comment
@richlander

richlander Dec 4, 2015

Member

Added a link to this JIT porting guidance to the contributing guidelines so that it isn't lost. This should be actually merged into the document at some point.

https://github.com/dotnet/coreclr/blob/master/Documentation/project-docs/contributing.md#contribution-guidelines

Member

richlander commented Dec 4, 2015

Added a link to this JIT porting guidance to the contributing guidelines so that it isn't lost. This should be actually merged into the document at some point.

https://github.com/dotnet/coreclr/blob/master/Documentation/project-docs/contributing.md#contribution-guidelines

@mikedn

This comment has been minimized.

Show comment
Hide comment
@mikedn

mikedn Dec 5, 2015

Contributor

This change will break the x86 build. It is not ready to merge until the legacy jit link errors are fixed.

In case you don't know yet: you need to add stackfp.cpp and codegenlegacy.cpp to ARCH_SOURCES to get the x86 legacy JIT to build.

Contributor

mikedn commented Dec 5, 2015

This change will break the x86 build. It is not ready to merge until the legacy jit link errors are fixed.

In case you don't know yet: you need to add stackfp.cpp and codegenlegacy.cpp to ARCH_SOURCES to get the x86 legacy JIT to build.

@jashook

This comment has been minimized.

Show comment
Hide comment
@jashook

jashook Dec 5, 2015

Contributor

Thank you for that information, I did not know that and was looking for it.

Contributor

jashook commented Dec 5, 2015

Thank you for that information, I did not know that and was looking for it.

@borgdylan

This comment has been minimized.

Show comment
Hide comment
@borgdylan

borgdylan Dec 6, 2015

Would the Ryujit32 bring-up involve supporting it on Linux as well?

Would the Ryujit32 bring-up involve supporting it on Linux as well?

@mikedn

This comment has been minimized.

Show comment
Hide comment
@mikedn

mikedn Dec 6, 2015

Contributor

@borgdylan Linux x86 support needs more than a x86 JIT compiler. And even the JIT might need some adjustments to work properly on Linux x86.

Contributor

mikedn commented Dec 6, 2015

@borgdylan Linux x86 support needs more than a x86 JIT compiler. And even the JIT might need some adjustments to work properly on Linux x86.

@borgdylan

This comment has been minimized.

Show comment
Hide comment
@borgdylan

borgdylan Dec 6, 2015

@mikedn I never questioned the fact hat a CLR has many moving parts ie. not just the JIT. What I wanted to know whether Linux x86 was on the near future road-map for CoreCLR.

@mikedn I never questioned the fact hat a CLR has many moving parts ie. not just the JIT. What I wanted to know whether Linux x86 was on the near future road-map for CoreCLR.

@jkotas

This comment has been minimized.

Show comment
Hide comment
@jkotas

jkotas Dec 6, 2015

Member

And even the JIT might need some adjustments to work properly on Linux x86.

The JIT should not need any significant adjustments (except for fixing build breaks). We should use the same custom managed x86 calling convention on Linux as is used on Windows. It was done for Silverlight on Mac x86 - it worked pretty well, the code that made it work is still there (e.g. https://github.com/dotnet/coreclr/blob/master/src/vm/fcall.h#L376).

Member

jkotas commented Dec 6, 2015

And even the JIT might need some adjustments to work properly on Linux x86.

The JIT should not need any significant adjustments (except for fixing build breaks). We should use the same custom managed x86 calling convention on Linux as is used on Windows. It was done for Silverlight on Mac x86 - it worked pretty well, the code that made it work is still there (e.g. https://github.com/dotnet/coreclr/blob/master/src/vm/fcall.h#L376).

@jkotas

This comment has been minimized.

Show comment
Hide comment
@jkotas

jkotas Dec 6, 2015

Member

whether Linux x86 was on the near future road-map for CoreCLR

It is on the road-map, but there is no set time line for when Microsoft folks may start working on it. We still have quite a bit work left to make Linux/MacOS x64 port great product.

@borgdylan I know you have done some work on the Linux x86 port a while ago. Any chance you could send PR with the changes you have so far? I will take any good changes that go towards the Linux x86 port. It does not (and should not) need to be one big PR that makes everything work.

Member

jkotas commented Dec 6, 2015

whether Linux x86 was on the near future road-map for CoreCLR

It is on the road-map, but there is no set time line for when Microsoft folks may start working on it. We still have quite a bit work left to make Linux/MacOS x64 port great product.

@borgdylan I know you have done some work on the Linux x86 port a while ago. Any chance you could send PR with the changes you have so far? I will take any good changes that go towards the Linux x86 port. It does not (and should not) need to be one big PR that makes everything work.

@mikedn

This comment has been minimized.

Show comment
Hide comment
@mikedn

mikedn Dec 6, 2015

Contributor

We should use the same custom managed x86 calling convention on Linux as is used on Windows.

Hmm, I was mostly thinking about the exception handling. I suppose Silverlight didn't use Windows 32 bit style exception handling on OS X. Or did it?

Contributor

mikedn commented Dec 6, 2015

We should use the same custom managed x86 calling convention on Linux as is used on Windows.

Hmm, I was mostly thinking about the exception handling. I suppose Silverlight didn't use Windows 32 bit style exception handling on OS X. Or did it?

@borgdylan

This comment has been minimized.

Show comment
Hide comment
@borgdylan

borgdylan Dec 6, 2015

@jkotas The repo has diverged a lot since then. All the work I did is now useless.

@jkotas The repo has diverged a lot since then. All the work I did is now useless.

@jkotas

This comment has been minimized.

Show comment
Hide comment
@jkotas

jkotas Dec 6, 2015

Member

Windows 32 bit style exception handling on OS X

The managed exception handling on Windows x86 is done via vectored exception handler, not via fs:[0] chains. The x86 JIT does not produce the Windows fs:[0] chains, so the same JITed code works fine on non-Windows x86 too.

Member

jkotas commented Dec 6, 2015

Windows 32 bit style exception handling on OS X

The managed exception handling on Windows x86 is done via vectored exception handler, not via fs:[0] chains. The x86 JIT does not produce the Windows fs:[0] chains, so the same JITed code works fine on non-Windows x86 too.

@mikedn

This comment has been minimized.

Show comment
Hide comment
@mikedn

mikedn Dec 6, 2015

Contributor

The managed exception handling on Windows x86 is done via vectored exception handler, not via fs:[0] chains.

Thanks, I completely forgot that. Now I feel really stupid, especially that I remember that I thought it's really cool that x86 CLR doesn't use fs:[0] chains when I first discovered it.

Contributor

mikedn commented Dec 6, 2015

The managed exception handling on Windows x86 is done via vectored exception handler, not via fs:[0] chains.

Thanks, I completely forgot that. Now I feel really stupid, especially that I remember that I thought it's really cool that x86 CLR doesn't use fs:[0] chains when I first discovered it.

@jashook jashook changed the title from Build the legacy jit for x86 and arm32 targets. to Build the legacy jit for x86. Dec 12, 2015

@jkotas

View changes

Show outdated Hide outdated src/jit/ryujit_x86/CMakeLists.txt
@@ -0,0 +1,46 @@
project(ryujit)

This comment has been minimized.

@jkotas

jkotas Dec 13, 2015

Member

This project name is used by jit/ryujit/standalone. It would be nice to use different name here.

@jkotas

jkotas Dec 13, 2015

Member

This project name is used by jit/ryujit/standalone. It would be nice to use different name here.

@jkotas

View changes

Show outdated Hide outdated src/jit/ryujit_x86/CMakeLists.txt
remove_definitions(-DFEATURE_MERGE_JIT_AND_ENGINE)
add_library(ryujit_x86

This comment has been minimized.

@jkotas

jkotas Dec 13, 2015

Member

We will need the same flavor for ARM too. Shouldn't this rather be called something like ryujit_newbackend or ryujit_unstable so that the same name works for both x86 and arm (and it is obvious from the name what it actually contains)?

@jkotas

jkotas Dec 13, 2015

Member

We will need the same flavor for ARM too. Shouldn't this rather be called something like ryujit_newbackend or ryujit_unstable so that the same name works for both x86 and arm (and it is obvious from the name what it actually contains)?

@jashook

This comment has been minimized.

Show comment
Hide comment
@jashook

jashook Dec 15, 2015

Contributor

@jkotas @BruceForstall @adiaaida @RussKeldorph PTAL. Verified that this successfully produces a statically linked legacy jit which fails 30 tests and protojit.dll to be used as the x86 bring up altjit.

Contributor

jashook commented Dec 15, 2015

@jkotas @BruceForstall @adiaaida @RussKeldorph PTAL. Verified that this successfully produces a statically linked legacy jit which fails 30 tests and protojit.dll to be used as the x86 bring up altjit.

@jkotas

View changes

Show outdated Hide outdated src/jit/CMakeLists.txt
@@ -137,3 +139,7 @@ set(CLR_EXPORTED_SYMBOL_FILE ${CLRJIT_EXPORTS_DEF})
add_subdirectory(dll)
add_subdirectory(crossgen)
add_subdirectory(standalone)
if (CLR_CMAKE_PLATFORM_ARCH_I386)

This comment has been minimized.

@jkotas

jkotas Dec 15, 2015

Member

It would be nice to make these I386 || ARM like you had it in your previous iterations...

@jkotas

jkotas Dec 15, 2015

Member

It would be nice to make these I386 || ARM like you had it in your previous iterations...

@@ -0,0 +1,46 @@
project(protojit)

This comment has been minimized.

@jkotas

jkotas Dec 15, 2015

Member

You should add .gitmirror file on the TFS into this directory first (since it exists on TFS side), wait for it to be mirrored and merged, and then commit this. The mirror is likely going to choke on it otherwise.

@jkotas

jkotas Dec 15, 2015

Member

You should add .gitmirror file on the TFS into this directory first (since it exists on TFS side), wait for it to be mirrored and merged, and then commit this. The mirror is likely going to choke on it otherwise.

This comment has been minimized.

@jkotas

jkotas Dec 15, 2015

Member

I have just added it for you, it will get propagated in a couple of minutes ... you do not need to worry about it.

@jkotas

jkotas Dec 15, 2015

Member

I have just added it for you, it will get propagated in a couple of minutes ... you do not need to worry about it.

@jkotas

This comment has been minimized.

Show comment
Hide comment
@jkotas

jkotas Dec 15, 2015

Member

LGTM otherwise.

Member

jkotas commented Dec 15, 2015

LGTM otherwise.

@dnfclas

This comment has been minimized.

Show comment
Hide comment
@dnfclas

dnfclas Dec 15, 2015

@jashook, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

dnfclas commented Dec 15, 2015

@jashook, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@dnfclas dnfclas added the cla-signed label Dec 15, 2015

@jkotas

View changes

Show outdated Hide outdated src/jit/dll/CMakeLists.txt
@@ -1,5 +1,9 @@
project(ClrJit)
if(CLR_CMAKE_PLATFORM_ARCH_I386)

This comment has been minimized.

@jkotas

jkotas Dec 15, 2015

Member

It may be a good idea to add this to jit/crossgen/CMakeLists.txt too so that both runtime and crossgen have the same flavor of the JIT statically linked in.

@jkotas

jkotas Dec 15, 2015

Member

It may be a good idea to add this to jit/crossgen/CMakeLists.txt too so that both runtime and crossgen have the same flavor of the JIT statically linked in.

@jkotas

This comment has been minimized.

Show comment
Hide comment
@jkotas

jkotas Dec 15, 2015

Member

Could you please rebase against current master and squash it into single commit, so that it is ready to merge?

Member

jkotas commented Dec 15, 2015

Could you please rebase against current master and squash it into single commit, so that it is ready to merge?

Build the legacy jit for x86 and arm.
Small changes to cmake to allow us to build the legacy jit as the fallback
jit for both x86 and arm.
@jashook

This comment has been minimized.

Show comment
Hide comment
@jashook

jashook Dec 15, 2015

Contributor

Done.

Contributor

jashook commented Dec 15, 2015

Done.

@jkotas

This comment has been minimized.

Show comment
Hide comment
@jkotas

jkotas Dec 15, 2015

Member

👍

Member

jkotas commented Dec 15, 2015

👍

jkotas added a commit that referenced this pull request Dec 15, 2015

@jkotas jkotas merged commit 4ab4ce8 into dotnet:master Dec 15, 2015

14 checks passed

CentOS7.1 x64 Debug Build Build finished. 803 tests run, 0 skipped, 0 failed.
Details
CentOS7.1 x64 Release Build Build finished. 803 tests run, 0 skipped, 0 failed.
Details
FreeBSD x64 Debug Build Build finished. 803 tests run, 0 skipped, 0 failed.
Details
FreeBSD x64 Release Build Build finished. 803 tests run, 0 skipped, 0 failed.
Details
OSX x64 Debug Build Build finished. 803 tests run, 0 skipped, 0 failed.
Details
OSX x64 Release Build Build finished. 803 tests run, 0 skipped, 0 failed.
Details
OSX x64 Release Build and Test Build finished. No test results found.
Details
Ubuntu x64 Debug Build Build finished. 803 tests run, 0 skipped, 0 failed.
Details
Ubuntu x64 Release Build Build finished. 803 tests run, 0 skipped, 0 failed.
Details
Ubuntu x64 Release Build and Test Build finished. No test results found.
Details
Windows_NT x64 Debug Build Build finished. 5011 tests run, 0 skipped, 0 failed.
Details
Windows_NT x64 Release Build Build finished. 5011 tests run, 0 skipped, 0 failed.
Details
Windows_NT x86 Debug Build Build finished. No test results found.
Details
Windows_NT x86 Release Build Build finished. No test results found.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment