-
Notifications
You must be signed in to change notification settings - Fork 5.5k
NativeAOT: Add win-x86 support #99372
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
Merged
MichalStrehovsky
merged 32 commits into
dotnet:main
from
filipnavara:not-a-win-x86-naot-001
Mar 11, 2024
Merged
Changes from all commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
8d01b3d
Fix AV checks
filipnavara 59d83fb
Implement RhpLockCmpXchg64
filipnavara 456e06e
Fix accessing ThunkParamSlot
filipnavara 2a0c256
Implement P/Invoke asm helpers
filipnavara 20fabf9
WIP: Math helpers
filipnavara e91de71
Update runtime tests to work in win-x86 configuration
filipnavara 79b6075
Enable win-x86 NativeAOT build
filipnavara ddf5055
Replace InternalCalls.memmove with Unsafe.CopyBlock
filipnavara 27da448
Use ref version of Unsafe.CopyBlock
filipnavara 7b664a1
Add support for P/Invoke resolution with mangled stdcall signatures
filipnavara 1a0e399
Fix build.
filipnavara 610d40c
Move SignatureBytes into Flags in MethodFixupCell
filipnavara cf69135
Attempt to remove the non-sense key in PInvokeLazyFixupFieldHashtable
filipnavara 392cce9
Cleanup the math helpers not to depend on CRT internals
filipnavara 1736ccf
RhCpuIdEx is FCall, fix the import declaration
filipnavara 8638f55
Make sure that thunk allocation functions can be resolved by the linker
filipnavara e5a4ca7
RhEnumerateConfigurationValues was incorrectly marked as FCall
filipnavara 514bee1
Fix Mac/iOS/tvOS build
filipnavara 1bf01b2
Add FCIMPL1_D/FCIMPL2_FF/FCIMPL2_DD macros and mark RhpDbl2ULng/RhpFl…
filipnavara 8ac691d
x86: Use FCalls for CRT math
filipnavara d6ca24f
s/REDHAWK_CALLCONV/F_CALL_CONV/g
filipnavara e168e24
Make RhCpuIdEx a QCall
filipnavara 44168a2
PR feedback: Move SignatureBytes computation and fix up Equals/CompareTo
filipnavara a0e279f
Use existing C helpers for Int/UInt/Long to Double conversions
filipnavara 880d97f
Update comment
filipnavara 37816ae
Apply suggestions from code review
filipnavara a8d355e
Apply suggestions from code review
filipnavara 30aae4b
Copy CoreCLR version of LMul.
filipnavara 543a4c8
Use consistent syntax
filipnavara b8c9ae0
Fix issue compiling some runtime tests for interop marshallers where …
filipnavara 03efa07
Update src/tests/nativeaot/SmokeTests/ControlFlowGuard/ControlFlowGua…
filipnavara 9dd4866
Use PInvokeLazyFixupFieldKey to pass NativeSignature
filipnavara File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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 like we can invert it
NativeAotUnsupportedOS; that set seems to be getting smaller. 😅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 would love to do that eventually;)
Uh oh!
There was an error while loading. Please reload this page.
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.
Seems like the unsupported OSes are
tizen,wasi, andbrowser. Not sure of the status forhaiku,illumos,solarisandnetbsdwhich happen to have some build system support.Unsupported archs are RV64, LA64, PowerPC (supported by Mono), WASM, and linux-x86. We may be able to shrink this list before .NET 9 ships.
Uh oh!
There was an error while loading. Please reload this page.
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.
Yup, something like:
usage:
Condition="'$(NativeAotUnsupportedOS)' != 'true'"and s390x.
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.
FWIW, this only needs to create a subset of what can already build with CoreCLR but does not build for native AOT (so that we don't break
./build.sh clrfor a platform that already works otherwise).I don't think browser/wasi/powerpc/s390 builds with CoreCLR, so we don't mind if native AOT makes
./build.sh clrmore broken. If someone brings up a new platform and wants native AOT out of the way, they can exclude it, same way they can exclude anything else they don't (yet) want as part of bringup.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.
Given we have to ensure that clr.iltools and clr.packages remain intact:
runtime/eng/pipelines/runtime-community.yml
Line 66 in 3eb8c7f
we might as well just list the 'not yet supported' ones without classifying them further.
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.
Clr.iltools "just works" because clr.iltools doesn't include coreclr VM and doesn't include any native AOT parts either, same for clr.packages.
Does building the clr subset (the entire clr subset, not just ilasm/dasm) work on s390 today? If not, I don't see why we'd need an extra exclusion for the native aot part. We shouldn't need to add extra exclusions (that are specific to native AOT) every time someone adds an obscure platform for mono. We either skip the entire clr build (i.e. we do the thing where build is skipped for all of coreclr, not just native AOT - except for iltools), or let it do whatever it does (break, probably). I think the latter is fine because that's how it is today.