-
Notifications
You must be signed in to change notification settings - Fork 197
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
Cross-architecture publishing on Linux #948
Cross-architecture publishing on Linux #948
Conversation
<!-- OSArchitecture does not report the true OS architecture for x86 and x64 processes running on Windows ARM64. --> | ||
<!-- The following condition checks those cases. --> | ||
<OSHostArch Condition="$([MSBuild]::IsOSPlatform('Windows')) and | ||
('$(PROCESSOR_ARCHITEW6432)' == 'ARM64' or Exists('$(SystemRoot)\System32\xtajit64.dll'))">arm64</OSHostArch> |
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.
We could read the true OS architecture from the registry; however, the .NET Core version of MSBuild comes with no registry provider, so we would have to resort to running reg query
or similar.
@@ -10,9 +10,8 @@ | |||
<DebugSymbols Condition="'$(DebugSymbols)' == ''">true</DebugSymbols> | |||
</PropertyGroup> | |||
|
|||
<!-- Part of workaround for lack of secondary build artifact import - https://github.com/Microsoft/msbuild/issues/2807 --> |
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.
Discussed removing these comments with A-And.
<IlcHostArch Condition="'$(IlcHostArch)' == ''">x64</IlcHostArch> | ||
|
||
<OSHostArch>$([System.Runtime.InteropServices.RuntimeInformation]::OSArchitecture.ToString().ToLowerInvariant)</OSHostArch> | ||
<!-- OSArchitecture does not report the true OS architecture for x86 and x64 processes running on Windows ARM64. --> |
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 do not think it is helpful to go out of your way to detect the underlying OS when the x64 emulator is trying really hard to make it impossible. If you are using x64 dotnet on ARM64 machine, I think we should use x64 ilc by default as well.
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.
when the x64 emulator is trying really hard to make it impossible
On the other hand, this might be considered as a shortcoming of RuntimeInformation.OSArchitecture
's implementation, which I reported in dotnet/runtime#26612. We could use IsWow64Process2
API or read this registry key:
$(Registry:HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\Environment@PROCESSOR_ARCHITECTURE)
which means the real OS architecture is not that hidden.
A more important scenario here is the x86 version of dotnet.exe
running on Windows ARM64 with no x64 emulation. In that case we cannot run the x64 version of ILCompiler. Regarding the x64 version of dotnet.exe
— yes, we could skip checking for xtajit64.dll
and run the x64 version of ILCompiler. As a side effect, that would require adding the runtime.win-x64.Microsoft.DotNet.ILCompiler
package reference to the project file when targeting win-arm64
.
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.
A more important scenario here is the x86 version of dotnet.exe running on Windows ARM64 with no x64 emulation.
It is the same as x86 version of dotnet.exe running on 32-bit Windows. I think we should just fail the compilation in this case by default. If somebody really needs to make that work, they can specify IlcHostArch
that they would like to use explicitly.
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.
It is the same as x86 version of dotnet.exe running on 32-bit Windows. I think we should just fail the compilation in this case by default.
Since ILCompiler is not tightly coupled to .NET SDK, I do not feel convinced that the architecture of dotnet.exe
or msbuild.exe
utilities should affect the architecture of ILCompiler we use. In fact, I accidentally made that change and immediately got bug report #933. Some developers use the x86-hosted dotnet.exe
to target win-x64 with Native AOT. And if we support the x86-on-x64 scenario, it is hard to justify not supporting the x86-on-ARM64 scenario, which employs the same WoW64 technology.
If somebody really needs to make that work, they can specify
IlcHostArch
that they would like to use explicitly.
Yes, but there is a discoverability issue. If x64 emulation is enabled, ILCompiler would run slower and people may not be aware why, especially if it is run under the hood of a complex build system, and just blame the company or the device. Of course, using the x64 version of dotnet.exe
or msbuild.exe
on ARM64 is not performance-wise in the first place; however, switching a big product to native execution on ARM64 is frequently a gradual process and some details may be out of the developer's control. Finally, we could make the IlcHostArch
property more discoverable by mentioning it in a Warning/Error message, but now that seems more complicated than the it-just-works approach.
Could you please also document the cross-compilation steps in https://github.com/dotnet/runtimelab/tree/feature/NativeAOT/docs/using-nativeaot ? |
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.
LGTM. Thanks!
Allow cross-architecture publishing on Linux by setting the
SysRoot
property. Includes some cleanup and formatting fixes.