Page MenuHomeFreeBSD

Disable LLD_IS_LD option combinations that fail
ClosedPublic

Authored by emaste on Feb 8 2017, 1:47 PM.

Details

Summary

If WITH_LLD is disabled LLD cannot be installed as /usr/bin/ld, so disable WITH_LLD_IS_LD.

Currently we do not compare the LLD host/in-tree version and LLD requires the LLVM libraries to be built, so force WITH_SYSTEM_COMPILER off when WITH_LLD_IS_LD is set.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

emaste updated this revision to Diff 24870.Feb 8 2017, 1:47 PM
emaste retitled this revision from to Disable unsupported LLD_IS_LD options.
emaste updated this object.
emaste edited the test plan for this revision. (Show Details)
emaste added reviewers: bdrewery, dim, lattera-gmail.com.
emaste added a comment.Feb 8 2017, 1:57 PM

We can use the same logic to avoid rebuilding LLD if the host version is the same as the in-tree version. Because LLD requires LLVM libraries I think we should just roll the eventual LLD version check into the existing MK_SYSTEM_COMPILER test in Makefile.inc1.

I'll test this patch sometime within the next 24-72 hours. As it stands, the logic looks completely fine to me. Thanks for working on this!

emaste retitled this revision from Disable unsupported LLD_IS_LD options to Disable LLD_IS_LD option combinations that fail.Feb 8 2017, 2:09 PM
emaste edited edge metadata.
lifanov added a subscriber: lifanov.Feb 8 2017, 8:07 PM
lifanov added inline comments.
share/mk/src.opts.mk
333 ↗(On Diff #24870)

collapse "yet yet"

Patch tested successfully. Other than the typo @lifanov noted, the patch looks good to me.

bdrewery accepted this revision.Feb 9 2017, 5:20 AM
bdrewery edited edge metadata.

We do still build lib/clang/libllvmminimal at least with SYSTEM_COMPILER triggered. What is missing from that?

This revision is now accepted and ready to land.Feb 9 2017, 5:20 AM
emaste added a comment.Feb 9 2017, 2:41 PM

I'm not sure of the minimal set of objects needed to build LLD but it's some subset of the combination of ${SRCS_MIN}, ${SRCS_MIW}, and ${SRCS_XDL} in lib/clang/libllvm. Perhaps later we can extend libllvmminimal to optionally include LLD's requirements.

This revision was automatically updated to reflect the committed changes.

I'm not sure of the minimal set of objects needed to build LLD but it's some subset of the combination of ${SRCS_MIN}, ${SRCS_MIW}, and ${SRCS_XDL} in lib/clang/libllvm. Perhaps later we can extend libllvmminimal to optionally include LLD's requirements.

I'd like to revert the MK_SYSTEM_COMPILER piece. It's unfortunate that it basically forces SYSTEM_COMPILER off for several architectures.

I tested reverting this and building arm64.aarch64 with SYSTEM_COMPILER triggered and everything built fine.

~/git/freebsd # /usr/obj/arm64.aarch64/root/git/freebsd/tmp/usr/bin/ld -V
LLD 4.0.0 (FreeBSD 297347) (compatible with GNU linkers)
~/git/freebsd # ls -al /usr/obj/arm64.aarch64/root/git/freebsd/tmp/usr/bin
total 23926
drwxr-xr-x   2 root  wheel        13 Apr 28 12:59 ./
drwxr-xr-x  13 root  wheel        13 Apr 28 12:36 ../
-rwxr-xr-x   1 root  wheel    822320 Apr 28 12:59 addr2line*
-rwxr-xr-x   1 root  wheel   1015352 Apr 28 12:59 ctfconvert*
-rwxr-xr-x   1 root  wheel    875240 Apr 28 12:59 ctfmerge*
lrwxr-xr-x   1 root  wheel         6 Apr 28 12:58 ld@ -> ld.lld
-rwxr-xr-x   1 root  wheel  38882448 Apr 28 12:58 ld.lld*
-rwxr-xr-x   1 root  wheel    847680 Apr 28 12:59 nm*
-rwxr-xr-x   2 root  wheel   1034184 Apr 28 12:59 objcopy*
-rwxr-xr-x   1 root  wheel    641328 Apr 28 12:59 size*
-rwxr-xr-x   1 root  wheel    635568 Apr 28 12:59 strings*
-rwxr-xr-x   2 root  wheel   1034184 Apr 28 12:59 strip*
-rwxr-xr-x   1 root  wheel    568552 Apr 28 12:59 sysinit*

libllvmminimal and libllvm were still built and lld was able to link and execute.

What problem was this trying to avoid?

Reverted in rS317608. This was a temporary measure to fix build breakage before rS316647 (LLD_BOOTSTRAP) went in.

It's unfortunate that it basically forces SYSTEM_COMPILER off for several architectures.

It should have affected only arm64.aarch64, no?