Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Port Mono to Raspberry Pi, ship as new linux-armv6 RID #62594
Port Mono to Raspberry Pi, ship as new linux-armv6 RID #62594
Changes from 42 commits
8589f9e
f8508a5
5545a71
c01ac90
2a32b83
eaae81e
7817d18
39a8287
aec66e9
aed2956
6c9c51d
1653948
cf3c2e3
e456e4a
f4cb981
31335fa
f661a0e
7ea9115
03f7fe4
bfa235a
e8c6ec3
9d40250
954d59c
d34a413
8f9b7d8
725e1e9
ef756dc
67f67d2
baf6cc8
a299408
f6b4be4
64ce768
80b7eaa
696efc8
6739626
5f43ec0
6ccd698
4fa9287
31c9691
a8329a3
d3b4f43
c82ed6e
3aa30cc
c26c029
9d5028c
7c4b261
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This is failing outerloop builds, e.g. https://dev.azure.com/dnceng/public/_build/results?buildId=1569496&view=logs&jobId=86b05f7f-a5b8-51f0-63db-a6d409e0da15&j=86b05f7f-a5b8-51f0-63db-a6d409e0da15&t=53a3521c-9616-50ba-cf1b-e3b67157a105
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.
Why?
In
Subsets.props
,<PrimaryRuntimeFlavor Condition="'$(TargetArchitecture)' == 's390x' or '$(TargetArchitecture)' == 'armv6'">Mono</PrimaryRuntimeFlavor>
should be settingPrimaryRuntimeFlavor
toMono
, then<DefaultCoreClrSubsets Condition="'$(PrimaryRuntimeFlavor)' != 'CoreCLR'">clr.iltools+clr.packages</DefaultCoreClrSubsets>
should be setting it so theclr
subset only builds the supported CoreCLR componentsI may be failing to parse how coreclr/ci.yml works.
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.
According to the message, this should be
#0-#6
:runtime/src/coreclr/pal/inc/unixasmmacrosarm.inc
Line 261 in 57bfe47
Don't know what would be the implication of changing
#7
to#6
. cc @janvorliAlternatively, we can skip armv6 here:
runtime/src/coreclr/pal/inc/unixasmmacrosarm.inc
Line 258 in 57bfe47
to get by the error.
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.
@directhex hmm... I think it's this template eng/pipelines/coreclr/templates/build-job.yml invoking src/coreclr/build-runtime.sh directly, not using subsets
runtime/eng/pipelines/coreclr/templates/build-job.yml
Lines 199 to 202 in 66a4f23
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 would be invalid change.
We should not be compiling this file for arm v6 at all. These .s files would require substantial changes to work correctly on arm v6.
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 think it is done this way to match s390x coverage (which builds il{d}asm tools along with mono). However, s390x required libunwind updates and https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/src/coreclr/pal/inc/unixasmmacross390x.inc to make those scenarios work. We would either need armv6 equivalent or disable
clr.iltools+clr.packages
subsets from building armv6. @directhex, per @jkotas' suggestion, we should disable those subsets armv6 for now (that will unblock the CI, outerloop legs).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.
clr.iltools
should not need any asm macros or libunwind to work. It is accidental dependency.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.
This PR made some assembly changes, those should be reverted too? I agree that
clr.iltools
alone does not require manyadd_subdirectory()
callssrc/coreclr/CMakeLists.txt
and changes injit
etc. which were made for armv6 and s390x shouldn't be needed.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 whole bunch of the iltools build fails without asm macros and libunwind, largely via coreclr pal/ and util/ dirs.