Skip to content

Conversation

@grendello
Copy link
Contributor

@grendello grendello commented Jan 28, 2022

Context: #6685
Context: fd5f31c

So far, Xamarin.Android has been using a GNU Binutils toolchain to compile and link native assembler code we generate
during application build. Unfortunately, Binutils have a problem with certain filename encodings on Windows and so we
decided to switch to an LLVM-based toolchain which handles such file names without issues.

However, since Mono/dotnet AOT expects a GNU build chain, it was necessary to implement a GNU Assembler (gas) wrapper
around the LLVM llvm-mc assembler, so that command lines used by the Mono AOT compiler keep working fine.

Since LLVM utilities are multi-target by default and, unlike GNU Binutils, they don't need separate builds of every utility,
we provide GNU Binutils architecture-prefixed wrapper scripts which invoke their LLVM counterparts with appropriate parameters.

The following LLVM utilities are included:

  • llvm-mc (assembler)
  • lld (linker)
  • llvm-strip

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

Might check if this fixes: #6685

And add proj.AotAssemblies = true; to that test?

@grendello grendello marked this pull request as ready for review January 31, 2022 15:22
@grendello grendello requested a review from jonpryor as a code owner January 31, 2022 15:22
@grendello
Copy link
Contributor Author

Might check if this fixes: #6685

And add proj.AotAssemblies = true; to that test?

Will do once this run's done, I want to see if there are any obvious issues.

@grendello
Copy link
Contributor Author

Some tests fail with the following error message:

obj/Release/aot/x86/UnnamedProject.dll/temp.s:20:2: error: unknown directive (TaskId:174)
  [aot-compiler stderr]         .hword 2

This is because llvm-mc doesn't support .hword (alias for `.short) on x86 targets. Fixing this will require changes to Mono/dotnet AOT probably

@grendello
Copy link
Contributor Author

Mono/dotnet AOT PRs:

dotnet/runtime#64561
mono/mono#21416

@grendello grendello requested a review from dellis1972 as a code owner January 31, 2022 21:39
@grendello grendello force-pushed the llvm-binutils branch 2 times, most recently from 47cdace to c333cff Compare February 1, 2022 18:44
grendello added a commit to grendello/xamarin-android that referenced this pull request Feb 7, 2022
Context: dotnet#6683 (comment)
Changes: mono/mono@a5d1934...148f536

  * mono/mono@148f536b0b4 [2020-02] [AOT] Use .short directive instead of .hword (#21419)
  * mono/mono@a6f3e8f179a Avoid an assert in ves_icall_RuntimeFieldInfo_SetValueInternal (#21420)
  * mono/mono@3c4f3de377d Add correct InetAccess category to HttpClientTest.Proxy_Disabled test and disable Ping tests
  * mono/mono@9f35bf1b807 Add missing handle function enter/return macros (#21407)
  * mono/mono@45efaa3b6f9 [interp] Remove hack for nint/nfloat (#21395)
grendello added a commit to grendello/xamarin-android that referenced this pull request Feb 7, 2022
Context: dotnet#6683 (comment)
Changes: mono/mono@a5d1934...148f536

  * mono/mono@148f536b0b4 [2020-02] [AOT] Use .short directive instead of .hword (#21419)
  * mono/mono@a6f3e8f179a Avoid an assert in ves_icall_RuntimeFieldInfo_SetValueInternal (#21420)
  * mono/mono@3c4f3de377d Add correct InetAccess category to HttpClientTest.Proxy_Disabled test and disable Ping tests
  * mono/mono@9f35bf1b807 Add missing handle function enter/return macros (#21407)
  * mono/mono@45efaa3b6f9 [interp] Remove hack for nint/nfloat (#21395)
grendello added a commit to grendello/xamarin-android that referenced this pull request Feb 7, 2022
Context: dotnet#6683 (comment)
Changes: mono/mono@a5d1934...148f536

  * mono/mono@148f536b0b4 [2020-02] [AOT] Use .short directive instead of .hword (#21419)
  * mono/mono@a6f3e8f179a Avoid an assert in ves_icall_RuntimeFieldInfo_SetValueInternal (#21420)
  * mono/mono@3c4f3de377d Add correct InetAccess category to HttpClientTest.Proxy_Disabled test and disable Ping tests
  * mono/mono@9f35bf1b807 Add missing handle function enter/return macros (#21407)
  * mono/mono@45efaa3b6f9 [interp] Remove hack for nint/nfloat (#21395)
jonpryor pushed a commit that referenced this pull request Feb 8, 2022
Context: #6683 (comment)

Changes: mono/mono@a5d1934...148f536

  * mono/mono@148f536b0b4: [2020-02] [AOT] Use .short directive instead of .hword (#21419)
  * mono/mono@a6f3e8f179a: Avoid an assert in ves_icall_RuntimeFieldInfo_SetValueInternal (#21420)
  * mono/mono@3c4f3de377d: Add correct InetAccess category to HttpClientTest.Proxy_Disabled test and disable Ping tests
  * mono/mono@9f35bf1b807: Add missing handle function enter/return macros (#21407)
  * mono/mono@45efaa3b6f9: [interp] Remove hack for nint/nfloat (#21395)
@grendello grendello force-pushed the llvm-binutils branch 2 times, most recently from 8bf9dec to a9132c4 Compare February 9, 2022 20:26
@grendello grendello requested a review from pjcollins as a code owner February 9, 2022 20:26
@grendello grendello marked this pull request as draft February 10, 2022 10:43
@grendello grendello force-pushed the llvm-binutils branch 3 times, most recently from 0b05031 to 164cc8a Compare February 11, 2022 10:13
@grendello grendello marked this pull request as ready for review February 11, 2022 13:00
@grendello
Copy link
Contributor Author

It's ready for review, but before merging the new toolchain has to be released (it's currently a pre-release) and that depends on this PR being green.

@grendello grendello changed the title [WIP] Use the new Xamarin.Android toolchain Use the new Xamarin.Android toolchain Feb 11, 2022
@grendello
Copy link
Contributor Author

The diacritics tests fails in the AOT compiler:

                     [AOT] response file obj\Release\aot\armeabi-v7a\Java.Interop.dll\response.txt: "--aot=asmwriter,mtriple=armv7-linux-gnueabi,tool-prefix=C:\a\_work\1\s\bin\Release\lib\xamarin.android\xbuild\Xamarin\Android\binutils\arm-linux-androideabi-,outfile=obj\Release\aot\armeabi-v7a\libaot-Java.Interop.dll.so,llvm-path=C:\a\_work\1\s\bin\Release\lib\xamarin.android\xbuild\Xamarin\Android,temp-path=obj\Release\aot\armeabi-v7a\Java.Interop.dll,ld-name=ld" "C:\a\_work\1\a\TestRelease\02-11_11.05.54\temp\SmokeTestBuildWithSpecialCharactersTrueTrue\テスト\obj\Release\android\assets\shrunk\Java.Interop.dll"
                      (TaskId:217)
                     [aot-compiler stderr] The specified response file can not be read (TaskId:217)
11:19:26.258   1:7>C:\a\_work\1\s\bin\Release\lib\xamarin.android\xbuild\Xamarin\Android\Xamarin.Android.Legacy.targets(692,5): error XA3001: Could not AOT the assembly: テスト.dll [C:\a\_work\1\a\TestRelease\02-11_11.05.54\temp\SmokeTestBuildWithSpecialCharactersTrueTrue\テスト\テスト.csproj]

@grendello
Copy link
Contributor Author

grendello commented Feb 11, 2022

It appears the toolchain works fine, I've created its new release and bumped this PR to use it. There were a couple of test failures here which appeared not to be related to the PR (one HTTP timeout and one dotnet run failure that did however build and install the app, logcat contained no errors neither did msbuild.binlog so I have no idea what went wrong, we'll see if it fails again)

@grendello grendello force-pushed the llvm-binutils branch 5 times, most recently from 5a116c1 to e2d0a94 Compare March 10, 2022 08:15
@grendello grendello force-pushed the llvm-binutils branch 4 times, most recently from 076efef to e6db87f Compare March 15, 2022 16:29
@grendello grendello marked this pull request as ready for review March 17, 2022 15:40
@grendello grendello changed the title Use the new Xamarin.Android toolchain Use new LLVM-based Xamarin.Android toolchain Mar 17, 2022
}
}

// DotNet fails, see https://github.com/xamarin/xamarin-android/issues/6685
Copy link
Contributor

Choose a reason for hiding this comment

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

This should instead link to: dotnet/runtime#65484

// Enable the commented out signature (and AOT) once the above is fixed
[Test]
[Category ("SmokeTests")]
// public void SmokeTestBuildWithSpecialCharacters ([Values (false, true)] bool forms, [Values (false, true)] bool aot)
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should instead do:

public void SmokeTestBuildWithSpecialCharacters ([Values (false, true)] bool forms, [Values (false /*, true */)] bool aot)

// TODO: AOT fails https://github.com/xamarin/xamarin-android/issues/6685
// .NET 6 uses AOT by default for Release
proj.AotAssemblies = false;
proj.AotAssemblies = false; /* = true; once AOT is fixed */
Copy link
Contributor

Choose a reason for hiding this comment

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

…and then set this to aot.

@jonpryor
Copy link
Contributor

jonpryor commented Mar 17, 2022

Context: fd5f31cc89648957243f31ba8fd7af7fccba11e4
Context: https://github.com/xamarin/xamarin-android/issues/6685
Context: https://github.com/xamarin/xamarin-android-binutils
Context: https://github.com/xamarin/xamarin-android-binutils/commit/6d4e3bbceff617235abf03727d5233cb97b80cfb
Context: https://github.com/xamarin/xamarin-android/issues/6840

Changes: https://github.com/xamarin/xamarin-android-binutils/compare/2.35.2-XA.1...L_13.0.1-4.0.1

So far, Xamarin.Android has been using a GNU Binutils toolchain to
compile and link native assembler code we generate during application
build.  Unfortunately, Binutils have a problem with certain filename
encodings on Windows -- which required that we downgrade to Binutils
2.35.2 in commit fd5f31cc -- so we decided to switch to an LLVM-based
toolchain which handles such file names without issues.

However, as mono/mono & dotnet/runtime AOT expects a
GNU Binutils-compatible toolchain, it was necessary to implement a
GNU Assembler (`gas`) wrapper around the LLVM `llvm-mc` assembler, so
that command lines used by the Mono AOT compiler keep working fine.

Since LLVM utilities are multi-target by default and, unlike GNU
Binutils, LLVM doesn't need separate builds of every utility, we now
provide GNU Binutils architecture-prefixed wrapper scripts which
invoke their LLVM counterparts with appropriate parameters.

The following LLVM utilities are included:

  - `llvm-mc` (assembler)
  - `lld` (linker)
  - `llvm-strip`

Migrating to LLVM from GNU Binutils increases our install size:

  * macOS `.pkg` size increases by ~29MB,
    installation size increases by ~75MB.
  * Windows `.vsix` size increases by ~14MB,
    installation size increases by ~40MB.

Aside: We've updated our unit tests to set
`$(_DisableParallelAot)`=True, because when AOT is done in parallel,
it's very difficult to make sense of the AOT compiler messages, as
messages from multiple processes are all intermixed.  We're disabling
parallel AOT to preserve our sanity when things break.

TODO: the Resulting `.apk` sizes also increase unexpectedly, with
`Xamarin.Forms_Performance_Integration-Signed-Release-Profiled-Aot.apkdesc`
showing a 1.7MB increase in `.apk` size.  We believe that this is
because of more verbose debug symbols.
xamarin/xamarin-android#6840 will track this.

grendello added a commit to grendello/xamarin-android that referenced this pull request Mar 17, 2022
Fixes: dotnet#6840
Context: dotnet#6683

Pass the `-s` flag to the native linker in order to produce shared
AOT libraries without debug symbols.  This is controlled by a new
msbuild property `$(AndroidStripAotLibraries)`, which defaults to
`true`
grendello added a commit to grendello/xamarin-android that referenced this pull request Mar 17, 2022
Fixes: dotnet#6840
Context: dotnet#6683

Pass the `-s` flag to the native linker in order to produce shared
AOT libraries without debug symbols.  This is controlled by a new
msbuild property `$(AndroidStripAotLibraries)`, which defaults to
`true`
grendello added a commit to grendello/xamarin-android that referenced this pull request Mar 17, 2022
Fixes: dotnet#6840
Context: dotnet#6683

Pass the `-s` flag to the native linker in order to produce shared
AOT libraries without debug symbols.  Symbols are stripped unless
the `$(DebugSymbols)` property is set to `True`.
@jonpryor jonpryor merged commit b21cbf9 into dotnet:main Mar 17, 2022
@grendello grendello deleted the llvm-binutils branch March 17, 2022 22:31
grendello added a commit to grendello/xamarin-android that referenced this pull request Mar 17, 2022
Fixes: dotnet#6840
Context: dotnet#6683

Pass the `-s` flag to the native linker in order to produce shared
AOT libraries without debug symbols.  Symbols are stripped unless
the `$(DebugSymbols)` property is set to `True`.
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants