Conversation
am11
requested review from
MichalStrehovsky,
steveisok and
vitek-karas
as code owners
ghost
added
the
dotnet-policy-service
bot
added
the
community-contribution
|
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
| <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> |
There was a problem hiding this comment.
Aligned with coreclr. Platforms is relevant in VS context.
This should be adding CoreCLR leg only. |
|
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. |
|
My point is that there should not be "Build linux-loongarch64 Release AllSubsets_Mono" leg in the first place. |
|
@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. |
|
If nothing else, it is misleading. Can we have a single loongarch leg that builds coreclr and corehost only (without mono)? |
|
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. |
|
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. |
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. |
|
FreeBSD is using such a template which excludes mono. I will switch to that one. It will help with #105004. |
|
I have excluded the packs subset from community platform which requires the live apphost. This PR is independent of the other two now. |
|
@jkotas, linux-loongarch/riscv64 and freebsd-x64 are on the same CI plan without mono. Please take a look. |
| { | ||
| __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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Changed to TODO linking #109276. This is @shushanhf's comment. CI cross build is not linking libatomic, so it should be ISA1.1.
jkotas
left a comment
jkotas
left a comment
There was a problem hiding this comment.
LGTM. Thanks!
@jkoritzinsky Could you please review the CI setup?
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
|
/ba-g Azure DevOps can't recover from restarts. |
3 participants
No description provided.