镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Add linux x86 build CI for pull request - #60817

Merged
jkotas merged 8 commits into
dotnet:mainfrom
clamp03:linux-x86
Nov 2, 2021
Merged

jkotas merged 8 commits into
dotnet:mainfrom
clamp03:linux-x86

Conversation

@clamp03

@clamp03 clamp03 commented Oct 25, 2021

Copy link
Copy Markdown
Member

No description provided.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Oct 25, 2021
@ghost

ghost commented Oct 25, 2021

Copy link
Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@jkotas jkotas added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 25, 2021
@jkotas

jkotas commented Oct 25, 2021

Copy link
Copy Markdown
Member

Looks like the current changes are breaking dotnet-runtime pipeline. Nothing from the pipeline run.

@clamp03
clamp03 force-pushed the linux-x86 branch 3 times, most recently from a52cf63 to c08b276 Compare October 25, 2021 13:05
@clamp03

clamp03 commented Oct 26, 2021 •

Copy link
Copy Markdown
Member Author

@jkotas @safern Could you please review again? I fixed some mistakes in the first commit and some build errors. I think CI failures are not caused by this PR.

@alpencolt @gbalykov It builds linux x86 with src/coreclr/build-runtime.sh checked x86 -cross -ci -clang9 -component runtime -component jit -component iltools and build.sh -subset clr.corelib -cross -arch x86 -c checked -ci commands. These builds only possible components for normal Linux x86 build . Please check these are good for our tizen build.
cc @wscho77 @HJLeee

@jkotas

jkotas commented Oct 26, 2021

Copy link
Copy Markdown
Member

The windows x86 legs are failing with unique errors. They may be related to this change somehow.

Comment thread src/coreclr/vm/CMakeLists.txt Resolved
@gbalykov

Copy link
Copy Markdown
Member

@clamp03 last time I checked (#50836 and 6.0-preview7), linux x86 build worked for next subsets: clr.runtime+clr.corelib+clr.iltools+clr.dactools+clr.jit, i.e.

ROOTFS_DIR=`pwd`/.tools/rootfs/x86/ ./build.sh --cross --arch x86 --runtimeConfiguration Release --librariesConfiguration Release --subset clr.runtime+clr.corelib+clr.iltools+clr.dactools+clr.jit

Has something changed?

@gbalykov gbalykov 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.

Ah, I see that CI uses build-runtime.sh script directly for runtime build instead of build.sh. Looks good.

@clamp03

clamp03 commented Oct 27, 2021

Copy link
Copy Markdown
Member Author

@gbalykov Thank you for the review.
@jkotas @safern Fixed an issue. All checks are passed. Could you please review it?
Thank you.

@safern safern 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.

Pipeline changes LGTM.

@clamp03
clamp03 force-pushed the linux-x86 branch 2 times, most recently from 1aeb75e to 2c0aa64 Compare October 28, 2021 05:53
@clamp03

clamp03 commented Oct 28, 2021

Copy link
Copy Markdown
Member Author

@safern Thank you!
@jkotas Fixed a conflict. Could you please review and merge if it is correct?

Comment thread src/coreclr/vm/CMakeLists.txt Outdated

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.

What is this change trying to fix?

@VSadov Could you please review this line?

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.

I am not sure I follow why this might be needed, on x86 specifically.

@VSadov VSadov Oct 28, 2021 •

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.

Normally we use it to avoid clash between two instances of gcdecode.cpp which both JIT and VM include, so when they are both statically linked into singlefile host, we need to pick just one.
And dac, I think needs gcdecode.cpp regardless of singlefile.

What is different on x86?

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.

@hoyosjs - does dac need gcdecode.cpp on x86 ?

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.

If it's used by the VM, most likely yes. The decoder only gets used in x86 looks like, and yeah - verified and there's calls to the functions in the decoder. SOS also uses a copy of it heavily.

@clamp03 clamp03 Oct 29, 2021 •

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.

Build error message which I try to fix is

>>            gcdump_dac.cpp.o:(.text+0x1D0) in archive ../../debug/daccess/libdaccess.a

ld.lld: error: duplicate symbol: decodeSigned(__ArrayDPtr<unsigned char const>, int*)
>>> defined at gcdecoder.cpp:64 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:64)
>>>            gcdecode.cpp.o:(decodeSigned(__ArrayDPtr<unsigned char const>, int*)) in archive ../../vm/libcee_dac.a
>>> defined at gcdecoder.cpp:64 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:64)
>>>            gcdump_dac.cpp.o:(.text+0x130) in archive ../../debug/daccess/libdaccess.a

ld.lld: error: duplicate symbol: decodeUDelta(__ArrayDPtr<unsigned char const>, unsigned int*, unsigned int)
>>> defined at gcdecoder.cpp:51 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:51)
>>>            gcdecode.cpp.o:(decodeUDelta(__ArrayDPtr<unsigned char const>, unsigned int*, unsigned int)) in archive ../../vm/libcee_dac.a
>>> defined at gcdecoder.cpp:51 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:51)
>>>            gcdump_dac.cpp.o:(.text+0xA0) in archive ../../debug/daccess/libdaccess.a

ld.lld: error: duplicate symbol: decodeUnsigned(__ArrayDPtr<unsigned char const>, unsigned int*)
>>> defined at gcdecoder.cpp:34 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:34)
>>>            gcdecode.cpp.o:(decodeUnsigned(__ArrayDPtr<unsigned char const>, unsigned int*)) in archive ../../vm/libcee_dac.a
>>> defined at gcdecoder.cpp:34 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:34)
>>>            gcdump_dac.cpp.o:(.text+0x0) in archive ../../debug/daccess/libdaccess.a

ld.lld: error: duplicate symbol: decodeCallPattern(int, unsigned int*, unsigned int*, unsigned int*, unsigned int*)
>>> defined at gcdecoder.cpp:261 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:261)
>>>            gcdecode.cpp.o:(decodeCallPattern(int, unsigned int*, unsigned int*, unsigned int*, unsigned int*)) in archive ../../vm/libcee_dac.a
>>> defined at gcdecoder.cpp:261 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:261)
>>>            gcdump_dac.cpp.o:(.text+0x670) in archive ../../debug/daccess/libdaccess.a

ld.lld: error: duplicate symbol: InfoHdrSmall::isHeaderMatch(InfoHdr const&) const
>>> defined at gcdecoder.cpp:466 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:466)
>>>            gcdecode.cpp.o:(InfoHdrSmall::isHeaderMatch(InfoHdr const&) const) in archive ../../vm/libcee_dac.a
>>> defined at gcdecoder.cpp:466 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:466)
>>>            gcdump_dac.cpp.o:(.text+0x6B0) in archive ../../debug/daccess/libdaccess.a

ld.lld: error: duplicate symbol: callCommonDelta
>>> defined at gcdecoder.cpp:505 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:505)
>>>            gcdecode.cpp.o:(callCommonDelta) in archive ../../vm/libcee_dac.a
>>> defined at gcdecoder.cpp:505 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:505)
>>>            gcdump_dac.cpp.o:(.rodata+0xB20) in archive ../../debug/daccess/libdaccess.a
ld.lld: error: duplicate symbol: callPatternTable
>>> defined at gcdecoder.cpp:534 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:534)
>>>            gcdecode.cpp.o:(callPatternTable) in archive ../../vm/libcee_dac.a
>>> defined at gcdecoder.cpp:534 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:534)
>>>            gcdump_dac.cpp.o:(.rodata+0xB30) in archive ../../debug/daccess/libdaccess.a

ld.lld: error: duplicate symbol: infoHdrShortcut
>>> defined at gcdecoder.cpp:279 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:279)
>>>            gcdecode.cpp.o:(infoHdrShortcut) in archive ../../vm/libcee_dac.a
>>> defined at gcdecoder.cpp:279 (/dotnet/jobs/ci/check/src/coreclr/inc/gcdecoder.cpp:279)
>>>            gcdump_dac.cpp.o:(.rodata+0x1A0) in archive ../../debug/daccess/libdaccess.a
clang: error: linker command failed with exit code 1 (use -v to see invocation)
dlls/mscordac/CMakeFiles/mscordaccore.dir/build.make:147: recipe for target 'dlls/mscordac/libmscordaccore.so' failed
make[3]: *** [dlls/mscordac/libmscordaccore.so] Error 1
CMakeFiles/Makefile2:4161: recipe for target 'dlls/mscordac/CMakeFiles/mscordaccore.dir/all' failed
make[2]: *** [dlls/mscordac/CMakeFiles/mscordaccore.dir/all] Error 2

@clamp03 clamp03 Oct 29, 2021 •

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.

For libmscordaccore build, cee_dac and daccess conflict by including same src/coreclr/inc/gcdecorder.cpp
I thought gcdecoder.cpp can be excluded because it is already included in daccess.

@clamp03 clamp03 Oct 29, 2021 •

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.

@VSadov I checked Include vm/gcdecode.cpp in only windows (exclude vm/gcdecode.cpp from linux-x86). Simple tests work in linux-x86.

I think that cee_dac doesn't need to include inc/gcdecoder.cpp in windows. I guess a compiler of windows can resolve this duplicated symbols well. Actually, with clang10 (and above), it compiles without the duplicated symbol errors in linux. (*. CI uses clang9)

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.

@clamp03 - yes on Windows compiler could be more tolerant to symbol duplication.
Did you get errors when you tried the change in 4147fb6 ?

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.

I didn't get any errors. Reverted the last commit. Now it includes gcdecode.cpp in only windows. Thank you!

@clamp03

clamp03 commented Nov 2, 2021

Copy link
Copy Markdown
Member Author

@jkotas @VSadov Could you please review again and then merge if it is good? Thank you.

@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!

@jkotas
jkotas merged commit bfcb542 into dotnet:main Nov 2, 2021
@clamp03
clamp03 deleted the linux-x86 branch November 2, 2021 04:34
@ghost ghost locked as resolved and limited conversation to collaborators Dec 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

community-contribution Indicates that the PR has been added by a community member NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants