-
Notifications
You must be signed in to change notification settings - Fork 206
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
Compile NativeAOT on x86 #1531
Compile NativeAOT on x86 #1531
Conversation
I need to uncomment parts of runtimelab/src/coreclr/nativeaot/Runtime/i386/WriteBarriers.asm Lines 223 to 236 in 67927bd
But still ILC fails to process |
@@ -1,4 +1,7 @@ | |||
// Licensed to the .NET Foundation under one or more agreements. | |||
// The .NET Foundation licenses this file to you under the MIT license. | |||
|
|||
// USE_GC_INFO_DECODER does not defined for Win-x86 in CoreCLR |
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.
This won't be enough. You will also need to define it when building RyuJIT.
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.
I assume this is NativeAOT specific customization to https://github.com/dotnet/runtimelab/blob/feature/NativeAOT/src/coreclr/jit/CMakeLists.txt
Also I did not see any reference to USE_GC_INFO_DECODER
in jit folder. Are you talking that I need to use FEATURE_EH_FUNCLETS
then how I make that change purely NativeAOT specific ?
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.
I assume this is NativeAOT specific customization to https://github.com/dotnet/runtimelab/blob/feature/NativeAOT/src/coreclr/jit/CMakeLists.txt
Yes. It becomes tricky because it would mean that the JIT binary used by CoreCLR will be different from the JIT binary used by AOT. It is actually a bunch of binaries since we build multiple variants to support cross-compilation. There are short-term hacks possible in this branch. Once/if we move NativeAOT to dotnet/runtime, we will need to be able to use the same JIT binary for both regular CoreCLR and NativeAOT. If we keep the ABI differences, we may need to replace the ifdefs with dynamic checks in the JIT. Other option is to just stick with the same ABI as CoreCLR and clean it up as necessary (ie avoid forking the ABI for NativeAOT).
Also I did not see any reference to USE_GC_INFO_DECODER in jit folder
JIT has JIT32_GCENCODER
. JIT32_GCENCODER = !USE_GC_INFO_DECODER
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.
I think I found a way how we can postpone decision temporary. Instead of this particular change, I remove these lines from .h files
runtimelab/src/coreclr/jit/jit.h
Lines 468 to 470 in 6f2cfcd
#ifdef TARGET_X86 | |
#define JIT32_GCENCODER | |
#endif |
and
runtimelab/src/coreclr/inc/gcinfodecoder.h
Lines 22 to 24 in 6f2cfcd
#if !defined(TARGET_X86) | |
#define USE_GC_INFO_DECODER | |
#endif |
And will control these defines from CMake files. That change maybe be submitted to Runtime repo I think, and all hacks would be in CMake files in NativeAOT repo. That way we can explore all 3 options without too much commitment to either of them, if I only apply CMake hacks locally. What do you think?
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.
I do not think that there is a big difference in controlling it in cmake files vs. in header files. Both of these are short-term hacks that would be ok in this branch, but not in dotnet/runtime. You end up with a JIT build that is different from regular CoreCLR JIT build in either case.
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.
Ok, will try that. I think about JIT as black box for now, so it was a scary change for me. I think I understand direction now.
@@ -172,7 +172,7 @@ REDHAWK_PALEXPORT int32_t PalGetModuleFileName(_Out_ const TCHAR** pModuleNameOu | |||
return 0; | |||
} | |||
|
|||
REDHAWK_PALEXPORT uint64_t __cdecl PalGetTickCount64() | |||
REDHAWK_PALEXPORT uint64_t REDHAWK_PALAPI PalGetTickCount64() |
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.
PalGetTickCount64 is defined as cdecl in C#. It has to match.
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.
REDHAWK_PALAPI defined as StdCall on non-Unix, and empty value on Unix. Does this mean, that I should change to StdCall in C#?
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.
You can add new RhpGetTickCount64
that is cdecl and call that from C# instead of PalGetTickCount64. And then this change can stay.
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.
So you propose declare RhpGetTickCount64
in c++, for which I would add corresponding C# function, and use that function instead of PalGetTickCount64
? I probably can just define __cdecl
to nothing for TARGET_X86
in that file then.
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.
Yes. There is number of methods like that (e.g. RhpReleaseCastCacheLock
). They are using __cdecl
already.
#if CALLDESCR_FPARGREGSARERETURNREGS | ||
out CommonInputThunkStub | ||
#else |
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.
I do not see why this change is needed.
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.
This is purely to satisfy style cop. I may suppress it on these code. It did not like comma before other parameters.
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.
Suppressing the warning would be better. The ifdefs do not make logical sense with this change.
This helper is only needed for |
There bunch of assertions with only 2 kinds of them during ILC run with latest changes. Not sure if this is indication that I have to do something extra, or this is indication that I do something wrong runtimelab/src/coreclr/jit/gcencode.cpp Line 4594 in 6f2cfcd
and runtimelab/src/coreclr/gcinfo/gcinfoencoder.cpp Line 1158 in 6f2cfcd
Last one has additional information in form of this text message |
At least runtimelab/src/coreclr/gcinfo/gcinfoencoder.cpp Line 1158 in 6f2cfcd
related to runtimelab/src/coreclr/jit/lower.cpp Line 1204 in 6f2cfcd
Which seems to be related to |
Let's start with IL
Which translated to following statements
Eventually it transformed to
And this is last state of this part of tree before it start asserting. I mark argument which lowering complains about.
My understanding that assert do not like that "local variable" passed as struct, and this should be "morphed" earlier. If morphing is indeed proper place for turning structs into I will look there. I also will re-read responsibilities of each phase, so maybe I will get better understanding what's going on. |
Has this pull request been merged into the master branch? |
@unofficialdev I was stuck with that. I do not have enough cycles to make that work properly, as such this is only half done. I still have appetite for that, just delay this excercise. If you want to pursue, you are free base that on current work, |
3906cb0
to
3c12c4b
Compare
This is retrofitting parts of dotnet/runtimelab#1531
This is retrofitting parts of dotnet/runtimelab#1531
This work should continue in dotnet/runtime repo. |
Why was |
It was an oversight. Would you like to submit a PR with a fix? |
Thanks: dotnet/runtime#79538 |
Could you tell us how you were able to achieve NativeAOT compilation in x86? I want to link a C++ static library to my x86 project. |
Hi, work for x86 continues in the runtime repo, not here in runtimelab. |
Thanks to suggestions from dotnet/runtime#86573 I was able to compile NativeAOT on Windows x86.
I apply just
USE_GC_INFO_DECODER
for now.