[mtouch] add mixed-mode support #4751
Merged
Conversation
This file contains 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
When enabling this option, mtouch will AOT compile `mscorlib.dll`. At runtime that means every method that wasn't AOT'd will be executed by the runtime interpreter.
lewurm
requested review from
chamons,
emaf,
jstedfast,
kzu and
VincentDondain
as code owners
rolfbjarne
reviewed
Sep 5, 2018
|
@rolfbjarne correct.
Note that |
|
OK, I think I understand, and I propose the following command-line syntax for mtouch:
Examples:
This means that your Does that sound like it could work (and be flexible enough)? |
|
I like your proposal, looks good. Do you mind to implement your suggestion yourself? For example, I don't know how the suggested option would propagate to msbuild task. |
|
Build failure Test results7 tests failed, 0 tests skipped, 75 tests passed.Failed tests
|
Yes, I can have a look at this. |
|
@lewurm have a look now to see if what I did will work for you. |
|
Build success |
spouliot
reviewed
Sep 7, 2018
tools/mtouch/mtouch.cs
Outdated
| { | ||
| app.UseInterpreter = true; | ||
| if (!string.IsNullOrEmpty (v)) { | ||
| app.InterpretedAssemblies.AddRange (v.Split (new char [] { ',' }, StringSplitOptions.RemoveEmptyEntries)); |
There was a problem hiding this comment.
minor: incorrect alignment
| @@ -1134,6 +1157,8 @@ void AOTCompile () | |||
| } | |||
|
|
|||
| if (App.UseInterpreter) | |||
| /* TODO: not sure? we might have to continue here, depending on | |||
| * the set of assemblies are AOT'd? */ | |||
There was a problem hiding this comment.
indeed, that condition is can't work if we're mixing
tools/common/Application.cs
Outdated
| @@ -64,6 +64,7 @@ public partial class Application | |||
| public bool? EnableCoopGC; | |||
| public bool EnableSGenConc; | |||
| public bool UseInterpreter; | |||
| public List<string> InterpretedAssemblies = new List<string> (); | |||
There was a problem hiding this comment.
hmm... it's inside common which is shared with XM
not sure it's the best place, Maybe under #if MTOUCH ?
There was a problem hiding this comment.
Fixed by moving the code to an mtouch-specific file.
| @@ -52,6 +52,7 @@ Copyright (C) 2013-2016 Xamarin. All rights reserved. | |||
| <MtouchProjectDirectory>$(MSBuildProjectDirectory)</MtouchProjectDirectory> | |||
| <MtouchEnableSGenConc Condition="'$(MtouchEnableSGenConc)' == ''">False</MtouchEnableSGenConc> | |||
| <MtouchUseInterpreter Condition="'$(MtouchUseInterpreter)' == ''">False</MtouchUseInterpreter> | |||
| <MtouchInterpreter Condition="'$(MtouchInterpreter)' == ''">all</MtouchInterpreter> | |||
There was a problem hiding this comment.
maybe both variables could be shared ? otherwise we end up with inconsistent state - and the tooling might start doing weird things (if the same duplicated conditions are not also duplicated in code)
There was a problem hiding this comment.
Fixed to use a single variable.
| @@ -832,6 +832,7 @@ Copyright (C) 2013-2016 Xamarin. All rights reserved. | |||
| EnableBitcode="$(MtouchEnableBitcode)" | |||
| EnableSGenConc="$(MtouchEnableSGenConc)" | |||
| UseInterpreter="$(MtouchUseInterpreter)" | |||
| Interpreter="$(MtouchInterpreter)" | |||
|
Build failure Test results147 tests failed, 0 tests skipped, 71 tests passed.Failed tests
|
… it a non-required property.
lewurm
commented
Sep 10, 2018
tools/mtouch/mtouch.cs
Outdated
| @@ -476,6 +477,8 @@ public static string GetAotArguments (Application app, string filename, Abi abi, | |||
| args.Append ("llvmonly,"); | |||
| else if (interp) | |||
There was a problem hiding this comment.
I suggest to add an assert here, interp should only be true of fname == "mscorlib.dll" is true.
There was a problem hiding this comment.
Don't you mean that interp_full should only be true if name == "mscorlib.dll"?
There was a problem hiding this comment.
No, other assemblies than mscorlib.dll are vaild with --aot=interp,full too, as it will add wrappers specific to signatures seen during AOT compilation that are necessary for the AOT<>interpreter transitions.
However, --aot=interp only makes sense for mscorlib.dll. At least that's how it's supposed to work, I should double check that on the mono side: mono/mono#10543
There was a problem hiding this comment.
Ah, I see your point, for other interpreted assemblies this method (GetAotArguments) shouldn't be called at all.
lewurm
commented
Sep 10, 2018
luhenry
changed the title
jstedfast
approved these changes
Sep 10, 2018
|
Build success |
|
Build success |
spouliot
pushed a commit
that referenced
this pull request
Oct 10, 2018
* Bump to mono:2018-06 * Bump mono * Updates compression to work with the public span * Bump mono * Fixes pointer check logic in Deflater * Bump mono * Fixes pointer check logic in Deflater * Bump mono * Bump Mono * [runtime] always use `mono_jit_set_aot_mode` (#4491) `mono_jit_set_aot_only` is deprecated and accidentally broke with mono/mono#7887 This should fix device tests with `mono-2018-06` * Testing with Zoltan's patch * Include libmono-system-native on Xamarin.Mac * Bump Mono Commit list for mono/mono: * mono/mono@7bcda192a06 Bump llvm to release_60/fc854b8ec5873d294b80afa3e6cf6a88c5c48886. (#9786). (#9804) * mono/mono@23e95ec7ad7 Apply F# portable pdb debug fix for pinvokes & bump (#9797) * mono/mono@295f6d32afd [2018-06] [MacOS] On Mac, use the copyfile API to copy files (#9696) Diff: mono/mono@7d5f4b6...7bcda19 * Revert 4bacab3, it doesn't fix the ios aot problems. * Bump mono * [tests] Adjust the MT0137 test for mcs change in behavior. Starting with mono 5.16 mcs will now add assembly references when the assembly is only used in attributes (this was already the case for csc in both 5.14 and 5.16, so it seems to be a compatibility change). Adjust the MT0137 test accordingly. * [msbuild] Fix parsing of json parser errors to handle trailing periods in the error message. Fixes this test: 1) Test Failure : Xamarin.iOS.Tasks.Bug60536.TestACToolTaskCatchesJsonException ColumnNumber Expected: 2 But was: 0 * Bump mono * [builds] Install the old llvm binaries into the LLVM36 directory and make the 32 bit builds use that. * Bump mono * Bump mono * [jenkins] Don't give VSTS a fake branch. (#4667) Something in VSTS changed, and now fake branch names don't work anymore. So instead use real branch names (and for pull requests I've created a 'pull-request' branch we can use). * Assembly.LoadFile accepts only absolute path * [linker] Add new Facade (System.Threading.Tasks.Extensions). Fixes these MTouch test failures: 1. Xamarin.Linker.SdkTest.iOS_Unified : Facades Expected: But was: < "System.Threading.Tasks.Extensions" > 2. Xamarin.Linker.SdkTest.tvOS : Facades Expected: But was: < "System.Threading.Tasks.Extensions" > 3. Xamarin.Linker.SdkTest.watchOS : Facades Expected: But was: < "System.Threading.Tasks.Extensions" > * [mono-sdks] Necessary changes to unify the LLVM provisioning for both iOS and Android. (#4732) * Bump Mono * [mtouch] add mixed-mode support (#4751) * [mtouch] add --interp-mixed option When enabling this option, mtouch will AOT compile `mscorlib.dll`. At runtime that means every method that wasn't AOT'd will be executed by the runtime interpreter. * [mtouch] Add support to --interpreter to list the assemblies to (not) interpret. * [msbuild] Simplify interpreter code to use a single variable. * Fix whitespace. * [mtouch] Move mtouch-specific code to mtouch-specific file. * [msbuild] An empty string is a valid value for 'Interpreter', so make it a non-required property. * [mtouch] Add sanity check for aot-compiling interpreted assemblies. * Bump Mono * [linker] Updates SDKs facades list * Bump mono * [msbuild] Adds facades which might override default nuget version to framework list The collision resolver task reads them from here https://github.com/dotnet/sdk/blob/master/src/Tasks/Common/ConflictResolution/FrameworkListReader.cs * Bump to a VSfM version that can build XM Classic projects.
4 tasks
spouliot
added a commit
that referenced
this pull request
Dec 6, 2018
* Bump to mono:2018-06 * Bump mono * Updates compression to work with the public span * Bump mono * Fixes pointer check logic in Deflater * Bump mono * Fixes pointer check logic in Deflater * Bump mono * Bump Mono * [runtime] always use `mono_jit_set_aot_mode` (#4491) `mono_jit_set_aot_only` is deprecated and accidentally broke with mono/mono#7887 This should fix device tests with `mono-2018-06` * Testing with Zoltan's patch * Include libmono-system-native on Xamarin.Mac * Bump Mono Commit list for mono/mono: * mono/mono@7bcda192a06 Bump llvm to release_60/fc854b8ec5873d294b80afa3e6cf6a88c5c48886. (#9786). (#9804) * mono/mono@23e95ec7ad7 Apply F# portable pdb debug fix for pinvokes & bump (#9797) * mono/mono@295f6d32afd [2018-06] [MacOS] On Mac, use the copyfile API to copy files (#9696) Diff: mono/mono@7d5f4b6...7bcda19 * Revert 4bacab3, it doesn't fix the ios aot problems. * Bump mono * [tests] Adjust the MT0137 test for mcs change in behavior. Starting with mono 5.16 mcs will now add assembly references when the assembly is only used in attributes (this was already the case for csc in both 5.14 and 5.16, so it seems to be a compatibility change). Adjust the MT0137 test accordingly. * [msbuild] Fix parsing of json parser errors to handle trailing periods in the error message. Fixes this test: 1) Test Failure : Xamarin.iOS.Tasks.Bug60536.TestACToolTaskCatchesJsonException ColumnNumber Expected: 2 But was: 0 * Bump mono * [builds] Install the old llvm binaries into the LLVM36 directory and make the 32 bit builds use that. * Bump mono * Bump to mono:2018-08 * Initialize Dependency Injector. * Fix typo * Fix llvm build * Reflect latest X509CertificateImpl changes * Use same compile flags also for link Linking can fail if the minimum versions are different * Bump mono * Bump mono * Assembly.LoadFile accepts only absolute path * Bump mono * [jenkins] Don't give VSTS a fake branch. (#4667) Something in VSTS changed, and now fake branch names don't work anymore. So instead use real branch names (and for pull requests I've created a 'pull-request' branch we can use). * Assembly.LoadFile accepts only absolute path * [linker] Add new Facade (System.Threading.Tasks.Extensions). Fixes these MTouch test failures: 1. Xamarin.Linker.SdkTest.iOS_Unified : Facades Expected: But was: < "System.Threading.Tasks.Extensions" > 2. Xamarin.Linker.SdkTest.tvOS : Facades Expected: But was: < "System.Threading.Tasks.Extensions" > 3. Xamarin.Linker.SdkTest.watchOS : Facades Expected: But was: < "System.Threading.Tasks.Extensions" > * [linker] Add new Facade (System.Threading.Tasks.Extensions). Fixes these MTouch test failures: 1. Xamarin.Linker.SdkTest.iOS_Unified : Facades Expected: But was: < "System.Threading.Tasks.Extensions" > 2. Xamarin.Linker.SdkTest.tvOS : Facades Expected: But was: < "System.Threading.Tasks.Extensions" > 3. Xamarin.Linker.SdkTest.watchOS : Facades Expected: But was: < "System.Threading.Tasks.Extensions" > * [tests] Reference GuiUnit_Net_4_5 using a project reference. This makes sure GuiUnit is built when the main project is built. * [mono-sdks] Necessary changes to unify the LLVM provisioning for both iOS and Android. (#4732) * Bump Mono * [mtouch] add mixed-mode support (#4751) * [mtouch] add --interp-mixed option When enabling this option, mtouch will AOT compile `mscorlib.dll`. At runtime that means every method that wasn't AOT'd will be executed by the runtime interpreter. * [mtouch] Add support to --interpreter to list the assemblies to (not) interpret. * [msbuild] Simplify interpreter code to use a single variable. * Fix whitespace. * [mtouch] Move mtouch-specific code to mtouch-specific file. * [msbuild] An empty string is a valid value for 'Interpreter', so make it a non-required property. * [mtouch] Add sanity check for aot-compiling interpreted assemblies. * Bump Mono * [linker] Updates SDKs facades list * Bump mono * [msbuild] Adds facades which might override default nuget version to framework list The collision resolver task reads them from here https://github.com/dotnet/sdk/blob/master/src/Tasks/Common/ConflictResolution/FrameworkListReader.cs * Bump mono to pick up hybrid suspend fixes Pick up the 2018-08 backport (mono/mono#10551) of mono/mono#10545 This should fix GC hangs when managed code runs on a GCD threadpool worker thread. * [builds] Fix target name in llvm36 provisioning package-llvm-llvm36-32 isn't defined in external/mono/sdks/builds/llvm.mk, but package-llvm36-llvm32 is. * Revert "[builds] Fix target name in llvm36 provisioning" This reverts commit b233837. * Revert "Fix llvm build" This reverts commit f668561. * [mono-sdks] Necessary changes to unify the LLVM provisioning for both iOS and Android. (#4732) * Bump mono * Bump mono * Revert "Use same compile flags also for link" This reverts commit cf20539. * Bump mono to pick up mono/mono@1a309a7 * [mmptest] System.Core doesn't depend on Mono.Posix in 2018-08 System.Core doesn't depend on Mono.Posix anymore since it's using the corefx implementation of pipes now mono/mono@0f0e318 * Bump mono and minimum system mono * [security]: Make `SecCertificate` work with the latest runtime code. * Fix the `NATIVE_APPLE_CERTIFICATE` logic; it should only be defined when using `XAMARIN_APPLETLS` and not on watch. * It is no longer allowed to construct `X509Certificate` from a native pointer, the `X509Certificate(IntPtr)` and `X509Certificate2(IntPtr)` constructors now throw an exception. Use `X509CertificateImplAppl` directly instead. * Bump mono * Bump VSmac min version to 7.7.0.1373 To pick up the fix for https://devdiv.visualstudio.com/DevDiv/_workitems/edit/658916/ Should fix the XM classic build failures * Revert "Bump VSmac min version to 7.7.0.1373" This reverts commit b2686bb. * Bump to a VSfM version that can build XM Classic projects. * Bump mono system dependency * Bump mono Pick up mono/mono@1644a1a0 - fix for mono/mono#10743 * Bump mono * Bump mono * [monotouch-test] Disable X509Certificate(byte[]) tests on watchOS (#4942) * [monotouch-test] Disable X509Certificate(byte[]) tests on watchOS * [tests] Add TestRuntime.AssertNotWatchOS() * fixup WatchOS build * Bump mono * Bump mono * [tests] Disable link-preserve-calendar-1 until we can upgrade it to be a Unified test. See #4596 (comment) for reference: mono's desktop API changed, the linker behavior the test is verifying is only expected when using XM's Mobile profile. * Bump mono * Revert "[monotouch-test] Disable X509Certificate(byte[]) tests on watchOS (#4942)" This reverts commit d003a9b. * Bump Mono. * [security]: `NATIVE_APPLE_CERTIFICATE` should now be defined on watchOS as well. * Mono 2018-08 requires macOS 10.9+, so Xamarin.Mac must as well. * Bump min mono version for XM system apps. Using a system mono < 5.18 results in a TypeLoadException: Could not resolve type with token 01000032 from typeref (expected class 'Mono.ISystemDependencyProvider' in assembly 'mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089') * Bump guiunit to get updated min macOS version. Commit list for mono/guiunit: * mono/guiunit@9f67042 Bump min macOS version to 10.9. * mono/guiunit@0c3159a [Harness] Fix exit code of 0 being reported in case of exceptions in the harness * mono/guiunit@9b7497c Merge pull request #15 from mono/fix-more-warnings * mono/guiunit@a264470 Fix more warnings * mono/guiunit@dd094e7 Merge pull request #14 from mono/fix-warnings * mono/guiunit@b3afede Fix build warnings Diff: mono/guiunit@1306b0d...9f67042 * [tests] More min macOS version setting to 10.9. * Remove 10.7 & 10.8 availability attributes, since they're redundant now. * Bump mono * [2018-08][watchos] Use mono_dangerous_add_raw_internal_call for watchOS icalls (#5030) * Add optional mono_dangerous_add_raw_internal_call to exports.t4 * Use mono_dangerous_add_raw_internal_call on watchOS for icall registration Internal calls added with mono_dangerous_add_raw_internal_call run in GC Unsafe mode under cooperative and hybrid suspend, whereas internal calls added with mono_add_internal_call run in GC Safe mode since mono/mono@5756ba4 in order for hybrid suspend to be a transparent replacement for preemptive suspend (the old default). The icalls in GC Unsafe mode have a responsibility not to block indefinitely without manually performing a thread state transition to GC Safe mode, and in return they avoid a thread state transition when the icall is invoked from a managed method. * [mmptest] Less hardcoding. * Bump minimum mono one that has 'mono_dangerous_add_raw_internal_call'. * Bump mono * Bump system mono dependency * Fixes building mono tests * [ImageCaptureCore] Remove redundant availability attribute. * [mtouch] Clear the MONO_THREADS_SUSPEND environment variable before calling the AOT compiler. Works around mono/mono#11765. Visual Studio for Mac may set MONO_THREADS_SUSPEND, and this ends up confusing the AOT compiler when compiling for watchOS. So unset the environment variable before calling the AOT compiler. Works around mono/mono#11765.
I passed --interpreter to additional mtouch args and aot limitations went away on physical device. am I able to use other examples in d16? for example I tested all of them. If I use --interpreter, it will interpret all assemblies no matter what parameter I provide to it! I provided some invalid names to it, and it's didn't show any kind of error to me. Thanks |
|
@ysmoradi sorry I missed your message. How did you verify that a specific assembly is interpreted or not? |
We show a warning in that case, not an error: xamarin-macios/tools/mtouch/Target.cs Line 305 in a2c80ed You can make it an error by adding |
I have an issue which results into JIT fallback & crash on iOS devices only. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Note: Requires
mono-2018-06to be merged, thus opened against it.When enabling this option, mtouch will AOT compile
mscorlib.dll. At runtime that means every method that wasn't AOT'd will be executed by the runtime interpreter.mono/mono#6860