Skip to content

Add linux-loongarch64 CI leg - #111086

Merged
jkotas merged 12 commits into
dotnet:mainfrom
am11:feature/ci/loongarch64-pr-validation
Jan 8, 2025
Merged

Add linux-loongarch64 CI leg#111086
jkotas merged 12 commits into
dotnet:mainfrom
am11:feature/ci/loongarch64-pr-validation

Conversation

@am11

@am11 am11 commented Jan 4, 2025

Copy link
Copy Markdown
Member

No description provided.

@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Jan 4, 2025
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

Comment thread src/native/corehost/test/CMakeLists.txt Outdated
@am11
am11 requested review from janvorli and jkoritzinsky January 6, 2025 11:15
<OutputPath>$(RuntimeBinDir)IL/</OutputPath>
<Configurations>Debug;Release;Checked</Configurations>
<Platforms>x64;x86;arm;armv6;arm64;riscv64;s390x;wasm;ppc64le</Platforms>
<Platforms>x64;x86;arm;arm64</Platforms>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Aligned with coreclr. Platforms is relevant in VS context.

@jkotas

jkotas commented Jan 6, 2025

Copy link
Copy Markdown
Member

linux-loongarch64 Release AllSubsets_Mono

This should be adding CoreCLR leg only.

@am11

am11 commented Jan 6, 2025

Copy link
Copy Markdown
Member Author

The failure is not from mono, it's from corehost tests as I mentioned above. I'm waiting for the new image build dotnet/dotnet-buildtools-prereqs-docker#1311. The caching in docker layer is not ideal so we have to go back and forth each time there is a new platform.

@jkotas

jkotas commented Jan 6, 2025

Copy link
Copy Markdown
Member

My point is that there should not be "Build linux-loongarch64 Release AllSubsets_Mono" leg in the first place.

@am11

am11 commented Jan 6, 2025

Copy link
Copy Markdown
Member Author

@jkotas the name of this leg is a bit misleading but it is basically validating the all native code is compiling including libs and corehost. We have the same setup for riscv. There is no problem in building native mono on loongarch as we can see in the logs.

@jkotas

jkotas commented Jan 6, 2025

Copy link
Copy Markdown
Member

If nothing else, it is misleading. Can we have a single loongarch leg that builds coreclr and corehost only (without mono)?

@jkotas

jkotas commented Jan 6, 2025

Copy link
Copy Markdown
Member

riscv64 should be on the same plan. I do not think we want to continue to invest into riscv mono port, for the same reason we have rejected loongarch64 mono port.

@am11

am11 commented Jan 6, 2025

Copy link
Copy Markdown
Member Author

Removing riscv mono support would be a step backward IMO, but nonetheless, it should be done in main as a separate PR. This PR is adding loongarch next to riscv to keep it in sync. Future infra refactoring can be applied to both at the same time.

@jkotas

jkotas commented Jan 6, 2025

Copy link
Copy Markdown
Member

Removing riscv mono support would be a step backward

It has been becoming more apparent to us (core .NET team) that we do not have capacity to maintain two completely different runtimes that are equivalent.

If you would like to remove riscv mono support in a separate PR, that's fine; but this PR should not be adding mono loongarch to the CI.

@am11

am11 commented Jan 6, 2025

Copy link
Copy Markdown
Member Author

FreeBSD is using such a template which excludes mono. I will switch to that one. It will help with #105004.

@am11

am11 commented Jan 7, 2025

Copy link
Copy Markdown
Member Author

I have excluded the packs subset from community platform which requires the live apphost. This PR is independent of the other two now.

@am11

am11 commented Jan 7, 2025

Copy link
Copy Markdown
Member Author

@jkotas, linux-loongarch/riscv64 and freebsd-x64 are on the same CI plan without mono. Please take a look.

Comment thread eng/Subsets.props
{
__int128_t iComparand = ((__int128_t)pComparandAndResult[1] << 64) + (uint64_t)pComparandAndResult[0];
// NOTE: for LoongArch64, it supports 128bits atomic from 3A6000-CPU which is ISA1.1's version.
// The LA64's compiler will translate the `__sync_val_compare_and_swap` into calling the libatomic's library interface to emulate

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does the build target ISA1.0 or ISA1.1 by default? If it targets ISA1.0 by default, this implementation is broken. This should be TODO instead of NOTE, ideally with a link to an issue that tracks fixing it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Changed to TODO linking #109276. This is @shushanhf's comment. CI cross build is not linking libatomic, so it should be ISA1.1.

Comment thread src/coreclr/nativeaot/Runtime/unix/UnixNativeCodeManager.cpp Outdated

@jkotas jkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks!

@jkoritzinsky Could you please review the CI setup?

Comment thread src/coreclr/nativeaot/Runtime/unix/UnixNativeCodeManager.cpp Outdated
Co-authored-by: Jan Kotas <jkotas@microsoft.com>

@jkoritzinsky jkoritzinsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Build changes LGTM

@jkotas

jkotas commented Jan 8, 2025

Copy link
Copy Markdown
Member

/ba-g Azure DevOps can't recover from restarts.

@jkotas
jkotas merged commit 043e60b into dotnet:main Jan 8, 2025
@am11
am11 deleted the feature/ci/loongarch64-pr-validation branch January 8, 2025 22:47
@github-actions github-actions Bot locked and limited conversation to collaborators Feb 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-loongarch64 area-NativeAOT-coreclr community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants