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.

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

[NativeAOT] Properly implement targeting iOS-like platforms rather than pretending to be macOS #87610

Closed
Tracked by #80905
ivanpovazan opened this issue Jun 15, 2023 · 14 comments · Fixed by #96090
Closed
Tracked by #80905
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI os-ios Apple iOS
Milestone

Comments

@ivanpovazan
Copy link
Member

Description

Initial support for targeting iOS-like platforms with NativeAOT introduced with: #81476 uses a workaround in JIT interface:

target.IsOSXLike ? CORINFO_OS.CORINFO_MACOS : CORINFO_OS.CORINFO_UNIX;

hardwiring the compilation as if the target platform is macOS which also brings along corresponding implications on the native side.

@ghost
Copy link

ghost commented Jun 15, 2023

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

Issue Details

Description

Initial support for targeting iOS-like platforms with NativeAOT introduced with: #81476 uses a workaround in JIT interface:

target.IsOSXLike ? CORINFO_OS.CORINFO_MACOS : CORINFO_OS.CORINFO_UNIX;

hardwiring the compilation as if the target platform is macOS which also brings along corresponding implications on the native side.

Author: ivanpovazan
Assignees: -
Labels:

os-ios, area-NativeAOT-coreclr

Milestone: 8.0.0

@ivanpovazan
Copy link
Member Author

/cc: @akoeplinger

@MichalStrehovsky
Copy link
Member

What problem is this causing? If there are ABI differences (that this bit controls in the JIT), I'd expect we wouldn't be able to run a MAUI app.

@jkotas
Copy link
Member

jkotas commented Jun 16, 2023

Is this issue just about renaming macOS to OSXLike in the JIT and JIT/EE interface?

@filipnavara
Copy link
Member

Does the flag control the ABI only, or can it have effect on something like the minimum expected architecture baseline? (which is different between M1 ARM and Axx ARM).

@MichalStrehovsky
Copy link
Member

Instruction sets are configured here; the JIT doesn't decide this:

if (targetOS == TargetOS.OSX)
{
// For osx-arm64 we know that apple-m1 is a baseline
instructionSetSupportBuilder.AddSupportedInstructionSet("apple-m1");
}
else
{
instructionSetSupportBuilder.AddSupportedInstructionSet("neon"); // Lower baselines included by implication
}

@ivanpovazan
Copy link
Member Author

@akoeplinger could you please add more context?

@akoeplinger
Copy link
Member

Is this issue just about renaming macOS to OSXLike in the JIT and JIT/EE interface?

That is an option, but when I initially did the iOS changes for NativeAOT I tried to add a new platform CORINFO_IOS etc but then hit some issues with the consistency checks here:

// compMatchedVM is set to true if both CPU/ABI and OS are matching the execution engine requirements
//
// Do we have a matched VM? Or are we "abusing" the VM to help us do JIT work (such as using an x86 native VM
// with an ARM-targeting "altjit").

Back then I didn't have time to investigate further so we went with pretending to target macOS for iOS.

@MichalStrehovsky
Copy link
Member

Back then I didn't have time to investigate further so we went with pretending to target macOS for iOS.

The question is if there's anything different for iOS that we would need a new value to distinguish it in RyuJIT. We distinguish it as TargetOS in the C# part of the compiler, but only because there's one spot in the LLVM object writer that puts this into the object file (99.9% of the compiler doesn't see it as a different OS). Is there a part of RyuJIT that needs to know?

@akoeplinger
Copy link
Member

As far as I can see the JIT would be fine with a generic "Apple" value, except for the compatibility checks. E.g. this one:

// MacOS x64 uses the Unix jit variant in crossgen2, not a special jit
info.compMatchedVM =
info.compMatchedVM && ((eeInfo->osType == CORINFO_UNIX) || (eeInfo->osType == CORINFO_MACOS));

I assume we also use the Unix jit variant for iOS x64 (Simulator) today so presumably it works fine.

If you think that we don't need this distinction in the JIT I can send a PR to rename OSX -> Apple.

@agocke agocke added this to AppModel Jun 21, 2023
@MichalStrehovsky
Copy link
Member

As far as I can see the JIT would be fine with a generic "Apple" value, except for the compatibility checks. E.g. this one:

That compatibility check also uses TargetOS::IsMacOSX above which is currently broken #87794 without anything noticing - I don't know what this actually does.

If you think that we don't need this distinction in the JIT I can send a PR to rename OSX -> Apple.

If we're only discussing renaming this within the JIT, this is now fully a JIT issue, so I'm moving this to that area and leaving the call on the JIT team.

@MichalStrehovsky MichalStrehovsky added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed area-NativeAOT-coreclr labels Jun 21, 2023
@ghost
Copy link

ghost commented Jun 21, 2023

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

Issue Details

Description

Initial support for targeting iOS-like platforms with NativeAOT introduced with: #81476 uses a workaround in JIT interface:

target.IsOSXLike ? CORINFO_OS.CORINFO_MACOS : CORINFO_OS.CORINFO_UNIX;

hardwiring the compilation as if the target platform is macOS which also brings along corresponding implications on the native side.

Author: ivanpovazan
Assignees: -
Labels:

area-CodeGen-coreclr, os-ios

Milestone: 8.0.0

@JulieLeeMSFT
Copy link
Member

cc @dotnet/jit-contrib. @jakobbotsch please take a look.

@jakobbotsch
Copy link
Member

If you think that we don't need this distinction in the JIT I can send a PR to rename OSX -> Apple.

Indeed I don't think we need the distinction. Feel free to submit a PR to rename the value. I'm going to move the issue to future since this is just a cosmetic change.

@jakobbotsch jakobbotsch modified the milestones: 8.0.0, Future Jul 18, 2023
@ivanpovazan ivanpovazan modified the milestones: Future, 9.0.0 Aug 17, 2023
akoeplinger added a commit to akoeplinger/runtime that referenced this issue Dec 16, 2023
Now that we use it on iOS/tvOS as well, rename the variables to match.

Fixes dotnet#87610
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Dec 16, 2023
jkotas pushed a commit that referenced this issue Dec 17, 2023
* Rename IsMacOS to IsAppleOS in the JIT

Now that we use it on iOS/tvOS as well, rename the variables to match.

Fixes #87610

* PR feedback, use IsApplePlatform instead.
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Dec 17, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI os-ios Apple iOS
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants