Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Enable building NativeAOT for Apple mobile platforms #81780

Merged
merged 16 commits into from
Feb 16, 2023
Merged

Enable building NativeAOT for Apple mobile platforms #81780

merged 16 commits into from
Feb 16, 2023

Conversation

akoeplinger
Copy link
Member

These changes allow the NativeAOT runtime to compile for and run on iOS, tvOS and MacCatalyst.

Contributes to #81024, to actually compile an app we still have a few more changes that need to be upstreamed.

/cc @dotnet/ilc-contrib @ivanpovazan

@akoeplinger
Enable building NativeAOT for Apple mobile platforms
268e281 
These changes allow the NativeAOT runtime to compile for and run on iOS, tvOS and MacCatalyst.

Contributes to dotnet#81024
@ghost ghost assigned akoeplinger Feb 7, 2023
@ghost
Copy link

ghost commented Feb 7, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

These changes allow the NativeAOT runtime to compile for and run on iOS, tvOS and MacCatalyst.

Contributes to #81024, to actually compile an app we still have a few more changes that need to be upstreamed.

/cc @dotnet/ilc-contrib @ivanpovazan

Author: akoeplinger
Assignees: akoeplinger
Labels:

area-NativeAOT-coreclr
Milestone: -

@akoeplinger akoeplinger mentioned this pull request Feb 7, 2023
Closed
4 tasks
@akoeplinger akoeplinger linked an issue Feb 7, 2023 that may be closed by this pull request
Enable building NativeAOT for Apple mobile platforms #81024
Closed
4 tasks
@akoeplinger akoeplinger removed a link to an issue Feb 7, 2023
Enable building NativeAOT for Apple mobile platforms #81024
Closed
4 tasks
@filipnavara
Copy link
Member

filipnavara commented Feb 8, 2023 •
edited
Loading

W.r.t. thunk mapping: I believe NativeAOT implements two strategies for how to generate these thunks. The current default (FEATURE_RX_THUNKS) generates code at runtime and would not work on iOS. The alternative code path (FEATURE_FIXED_POOL_THUNKS) works by precompiling the thunk code and then mapping it multiple times (IIRC; @jkotas, @MichalStrehovsky feel free to correct me). I don't think the alternative code path is enabled anywhere (including this PR) and/or tested. We should probably open a separate issue to track that.

@ivanpovazan
Copy link
Member

W.r.t. thunk mapping: I believe NativeAOT implements two strategies for how to generate these thunks. The current default (FEATURE_RX_THUNKS) generates code at runtime and would not work on iOS. The alternative code path (FEATURE_FIXED_POOL_THUNKS) works by precompiling the thunk code and then mapping it multiple times (IIRC; @jkotas, @MichalStrehovsky feel free to correct me). I don't think the alternative code path is enabled anywhere (including this PR) and/or tested. We should probably open a separate issue to track that.

Good point! My understanding is that FEATURE_FIXED_POOL_THUNKS is mainly used for supporting marshalled delegates. Therefore, there is a "dedicated task": Marshalled delegates vs function pointers support with NativeAOT in the top-level tracking issue for NativeAOT on iOS work (Priority1 section). However, there is still no separate-dedicated issue for it, as we wanted to get some initial bits first. Nevertheless, if you think this should be escalated, we can open a tracking issue for it asap.

@filipnavara
Copy link
Member

My understanding is that FEATURE_FIXED_POOL_THUNKS is mainly used for supporting marshalled delegates.

Yes. It's used for delegates that have closure over some variables and need an associated context to reference the closure object, and the delegate is passed to native code. There's some test for it in src/tests/nativeaot/SmokeTests/PInvoke IIRC.

Therefore, there is a "dedicated task": Marshalled delegates vs function pointers support with NativeAOT in the #80905 (Priority1 section). However, there is still no separate-dedicated issue for it, as we wanted to get some initial bits first. Nevertheless, if you think this should be escalated, we can open a tracking issue for it asap.

I think it may be worth creating an issue so that the knowledge about FEATURE_FIXED_POOL_THUNKS is not lost in unrelated PRs/issues. I mainly mentioned it because this PR touched code paths in FEATURE_RX_THUNKS that would not really be usable long term...

@filipnavara
Copy link
Member

filipnavara commented Feb 9, 2023 •
edited
Loading

Turns out there are actually three different code paths for the thunks:

  • FEATURE_RX_THUNKS where thunk pages are generated at runtime (cannot work on iOS)
  • FEATURE_FIXED_POOL_THUNKS where the thunks are pre-generated but limited to certain number at compile time (used by some unofficial console ports)
  • If neither of the feature flags is set, then it runs in the mode with a pre-generated thunk page that is mapped multiple times

Currently there's no Unix code for anything but the FEATURE_RX_THUNKS mode. I wrote a small experimental app that mmaped its own executable page and run a code from it. It works on macOS/arm64 without any of the MAP_JIT flags, so it's likely work on iOS too. That makes the last mode look like the most viable one.

UPD: I updated my prototype to generate the static thunks, on macOS it passes the PInvoke test, so that's a good start.

janvorli
janvorli reviewed Feb 14, 2023
View reviewed changes
src/coreclr/gc/unix/gcenv.unix.cpp Show resolved Hide resolved
src/coreclr/pal/src/thread/process.cpp Outdated Show resolved Hide resolved
src/coreclr/inc/executableallocator.h Outdated Show resolved Hide resolved
@ivanpovazan ivanpovazan mentioned this pull request Feb 14, 2023
Closed
akoeplinger
akoeplinger commented Feb 14, 2023
View reviewed changes
src/coreclr/dlls/mscoree/coreclr/CMakeLists.txt
@@ -167,7 +167,7 @@ if(FEATURE_MERGE_JIT_AND_ENGINE)
set(CLRJIT_STATIC clrjit_static)
endif(FEATURE_MERGE_JIT_AND_ENGINE)

if (CLR_CMAKE_TARGET_OSX OR CLR_CMAKE_TARGET_MACCATALYST)
if (CLR_CMAKE_TARGET_OSX)
Copy link
Member Author

@akoeplinger akoeplinger Feb 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was erroneously added in 45887d1, MacCatalyst isn't building this piece.

janvorli
janvorli reviewed Feb 14, 2023
View reviewed changes
src/coreclr/gc/gc.cpp Show resolved Hide resolved
src/coreclr/gc/unix/gcenv.unix.cpp Show resolved Hide resolved
src/native/corehost/apphost/standalone/CMakeLists.txt Outdated Show resolved Hide resolved
@filipnavara filipnavara mentioned this pull request Feb 14, 2023
Merged
janvorli
janvorli approved these changes Feb 15, 2023
View reviewed changes
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@build-analysis build-analysis bot mentioned this pull request Feb 15, 2023
Closed
@akoeplinger akoeplinger merged commit e56db76 into dotnet:main Feb 16, 2023
@akoeplinger akoeplinger deleted the nativeaot-mobile branch February 16, 2023 01:00
@ghost ghost locked as resolved and limited conversation to collaborators Mar 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@marek-safar marek-safar marek-safar left review comments

@filipnavara filipnavara filipnavara left review comments

@jkotas jkotas jkotas left review comments

@kotlarmilos kotlarmilos kotlarmilos left review comments

@ivanpovazan ivanpovazan ivanpovazan left review comments

@janvorli janvorli janvorli approved these changes

@MichalStrehovsky MichalStrehovsky Awaiting requested review from MichalStrehovsky MichalStrehovsky is a code owner

Assignees

@akoeplinger akoeplinger

Labels
area-NativeAOT-coreclr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@akoeplinger @filipnavara @ivanpovazan @marek-safar @jkotas @janvorli @kotlarmilos @MichalStrehovsky