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

NativeAOT ARM64 libraries testing #64373

Merged
merged 9 commits into from
Feb 8, 2022
Merged

NativeAOT ARM64 libraries testing #64373

merged 9 commits into from
Feb 8, 2022

Conversation

MichalStrehovsky
Copy link
Member

Enable ARM64 libraries testing for NativeAOT.

  • Copy the crosstargeting build approach from crossgen2. We'll now build two ILC compilers when targeting ARM64 from x64: an arm64-hosted one that is useless on the build machine, and a x64-hosted one, that will work on the build machine. The ILCompiler.csproj/props/_crossarch.csproj projects mirror crossgen's approach.
  • Limit the number of libraries tests to one. Hopefully it avoids the build races that we'd see until we can update the repo build to .NET 7 Preview 1.
  • Create an ARM64 NativeAOT CI leg that sends the libraries tests to Helix to run.

@MichalStrehovsky
NativeAOT ARM64 libraries testing
afdd95f 
Enable ARM64 libraries testing for NativeAOT.

* Copy the crosstargeting build approach from crossgen2. We'll now build two ILC compilers when targeting ARM64 from x64: an arm64-hosted one that is useless on the build machine, and a x64-hosted one, that will work on the build machine. The `ILCompiler.csproj/props/_crossarch.csproj` projects mirror crossgen's approach.
* Limit the number of libraries tests to one. Hopefully it avoids the build races that we'd see until we can update the repo build to .NET 7 Preview 1.
* Create an ARM64 NativeAOT CI leg that sends the libraries tests to Helix to run.
agocke
agocke reviewed Feb 4, 2022
View reviewed changes
Copy link
Member

@agocke agocke left a comment

Choose a reason for hiding this comment

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

Generally looks good, just a few questions about how the cross-arch stuff is supposed to work

eng/testing/tests.singlefile.targets
@@ -22,11 +22,15 @@

<PropertyGroup Condition="'$(TestNativeAot)' == 'true'">
<IlcToolsPath>$(CoreCLRILCompilerDir)</IlcToolsPath>
<IlcToolsPath Condition="'$(TargetArchitecture)' != '$(BuildArchitecture)'">$(CoreCLRCrossILCompilerDir)</IlcToolsPath>
<CppCompilerAndLinker Condition="'$(TargetArchitecture)' != '$(BuildArchitecture)' and '$(HostOS)' != 'windows'">clang-9</CppCompilerAndLinker>
Copy link
Member

Choose a reason for hiding this comment

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

Do we have a variable for this? I can't remember -- maybe only in CMake, since that's where we do all the native compilation otherwise.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we only pass it directly to the native build scripts as the -clang9 argument. The managed build doesn't care about it. I could introduce a new property and pass it all the way from the YAML but it's unclear it would be an improvement - it's not a variable in YAML either so if this ever changes, we'll have to troubleshoot the CI either way.

src/coreclr/tools/aot/ILCompiler/ILCompiler_crossarch.csproj
@@ -0,0 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand the purpose of the crossarch project

Copy link
Member Author

Choose a reason for hiding this comment

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

This builds a version of ILC that is x64 hosted, even when we otherwise target arm or arm64. It matches what ../crossgen2/crossgen2_crossarch.csproj does. When we're crosstargeting to arm64, we wouldn't be able to run the ilc that we just built because it's for arm64 but the build machine is x64.

The ugly hardcoding of x64 matches what crossgen2 does - it captures the reality that when we're crossbuilding to arm64, the host is x64. I guess I could use HostArch, but this is a copy paste from crossgen2_crossarch.csproj.

src/coreclr/tools/aot/ILCompiler/ILCompiler_crossarch.csproj Outdated
@@ -0,0 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<CrossHostArch>x64</CrossHostArch>
Copy link
Member

Choose a reason for hiding this comment

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

Always x64? We don't have to worry about arm64?

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed to $(BuildArchitecture) to futureproof for the day when we crossbuild from a non-x64 host operating system. Also updated in the crossgen2 project.

agocke
agocke approved these changes Feb 7, 2022
View reviewed changes
Copy link
Member

@agocke agocke left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@agocke
Copy link
Member

agocke commented Feb 7, 2022

Although, staging is failing -- might be unrelated, but probably worth an investigation.

@MichalStrehovsky
Copy link
Member Author

Although, staging is failing -- might be unrelated, but probably worth an investigation.

The Build Browser wasm windows Release LibraryTests_AOT leg is failing because we're trying to run Windows batch scripts on a Linux machine (the backslashes in the path get eaten because they're invalid escape sequences, but the .cmd extension is telling):

========================== Starting Command Output ===========================
/bin/bash --noprofile --norc /mnt/vss/_work/_temp/f117afb7-9ea9-4154-94da-fcc36147a110.sh
/mnt/vss/_work/_temp/f117afb7-9ea9-4154-94da-fcc36147a110.sh: line 1: /mnt/vss/_work/1/sengcommoninit-tools-native.cmd: No such file or directory

Doesn't look related to this pull request. Cc @radical for odd WASM pipeline run.

@radical
Copy link
Member

radical commented Feb 7, 2022

re:wasm, I'm not sure what happened here. Like you said, it is using a linux image - https://dev.azure.com/dnceng/public/_build/results?buildId=1596254&view=logs&j=55f4943c-f76d-585b-7250-deab324f0a54&t=dcc862e9-ab6a-417b-b42c-fd7ec9c6c2ea

1ES Hosted Pool
SKU: Standard_D4_v3
Image: Build.Ubuntu.1804.Amd64.Open
...

Another windows job in the same build (Build Browser wasm windows release LibraryTests) seems to have run correctly though:

1ES Hosted Pool
SKU: Standard_D4_v3
Image: Build.Windows.10.Amd64.VS2019.Open
Image Version: 2022.0118.232525
...

@akoeplinger any ideas?

@akoeplinger
Copy link
Member

@radical it looks like that build job shouldn't have run on a Linux image since it's building a Windows config: Build Browser wasm windows Release LibraryTests_AOT

@radical
Copy link
Member

radical commented Feb 8, 2022

@radical it looks like that build job shouldn't have run on a Linux image since it's building a Windows config: Build Browser wasm windows Release LibraryTests_AOT

Any idea what might have caused this? Anything that we need to do?

@akoeplinger
Copy link
Member

It looks very weird. If I look at the expanded .yml via https://dev.azure.com/dnceng/public/_apis/build/builds/1596254/logs?logid=1 the job demands the Windows image:

    displayName: Build Browser wasm windows Release LibraryTests_AOT
    pool:
      name: NetCore1ESPool-Public
      demands:
      - ImageOverride -equals Build.Windows.10.Amd64.VS2019.Open

If it doesn't happen again I'd say it was a transient blip in the 1ES pool provider?

@MichalStrehovsky MichalStrehovsky merged commit b11469f into dotnet:main Feb 8, 2022
@MichalStrehovsky MichalStrehovsky deleted the arm64testing branch February 8, 2022 04:33
@ghost ghost locked as resolved and limited conversation to collaborators Mar 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@agocke agocke agocke approved these changes

Assignees

@MichalStrehovsky MichalStrehovsky

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

Successfully merging this pull request may close these issues.
4 participants
@MichalStrehovsky @agocke @radical @akoeplinger