Page MenuHomeFreeBSD

Rework WITHOUT_LLD/TOOLCHAIN fix from r327892 for cross-tools.
ClosedPublic

Authored by bdrewery on Jun 15 2018, 10:37 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 9 2024, 7:24 AM
Unknown Object (File)
Jan 9 2024, 7:24 AM
Unknown Object (File)
Jan 9 2024, 7:24 AM
Unknown Object (File)
Jan 9 2024, 7:24 AM
Unknown Object (File)
Jan 9 2024, 6:59 AM
Unknown Object (File)
Dec 24 2023, 11:36 PM
Unknown Object (File)
Dec 20 2023, 4:51 AM
Unknown Object (File)
Oct 28 2023, 1:26 PM
Subscribers

Details

Summary

Also correct an invalid check on MK_LLD_IS_LD for determining which
sources to link into libllvm. If lld is being built at all these
sources are needed. Having a link is irrelevant.

MK_LLD is for the installed lld while MK_LLD_BOOTSTRAP is for the build
tool. For WITH_SYSTEM_LINKER it is necesarry to separate the logic of
these two. When building libllvm TOOLS_PREFIX will be defined and
MK_LLD_BOOTSTRAP should be checked instead.

Sponsored by: Dell EMC

Test Plan

Did various WITHOUT_LLD and WITHOUT_LLD_IS_LD tests (with WITH_LLD_BOOTSTRAP set)

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Would it make sense to set some new variable based on TOOLS_PREFIX, MK_LLD, MK_LLD_BOOTSTRAP so that the same expression doesn't need to be repeated?

lib/clang/libllvm/Makefile
1278–1279 ↗(On Diff #43855)

I'm not sure what's going on here. What is SRCS_MIW? Should this really be MK_LLD ?

lib/clang/libllvm/Makefile
1278–1279 ↗(On Diff #43855)

Or maybe it needs to be similar to the checks below based on TOOLS_PREFIX. Like does it actually matter if ld is lld here despite me thinking it didn't?

lib/clang/libllvm/Makefile
1278–1279 ↗(On Diff #43855)

Introduced in D8879,

Use SRC_MIW to indicate minimal sources for the "world" stage.
Similarly, SRC_XDW for world with clang extras and lldb.

lib/clang/libllvm/Makefile
1278–1279 ↗(On Diff #43855)

SRC_MIW was D8879, MK_LLD_IS_LD comes from D9226, which is a leftover from earlier when we had only MK_LLD and MK_LLD_IS_LD.

I think your version (!TOOLS_PREFIX || MK_LLD_BOOTSTRAP) is correct.

lib/clang/libllvm/Makefile
1278–1279 ↗(On Diff #43855)

Well what is so special about SRCS_MIW for world?

The point of SYSTEM_LINKER is that /usr/bin/ld should match exactly what is used in WORLDTMP, so it can cross-link all lld-safe archs.
So this "special world" var list makes me wonder if it should really always be built for MK_LLD, regardless of whether it is the bootstrap linker or not.

lib/clang/libllvm/Makefile
1278–1279 ↗(On Diff #43855)

These used to be in SRCS_EXT for CLANG_EXTRAS but were enabled once llvm-objdump was enabled by default and at the time *not* for the cross-linker in WORLDTMP but only for the installed version.
rS310775 D8879

+.if !defined(TOOLS_PREFIX)
+SRCS_ALL+=     ${SRCS_MIW}
+.endif

So now I'm not sure why W was chosen since they are explicitly *not* added in for the *world* stage (when TOOLS_PREFIX is defined)

I'm going to see exactly which of these lld needs and move them to a different list that is more proper (not based on just LLD_BOOTSTRAP)

lib/clang/libllvm/Makefile
1278–1279 ↗(On Diff #43855)

Yes, this was split off so llvm-objdump could be built by default. SRCS_MIW was just a symbolic choice, I tried to keep to three-letter acronyms, but maybe "world" was not the right one. The idea is that you probably don't need any objdump binary as a cross-tool, but you only need it for the 'final' world build, so it can be installed.

lib/clang/libllvm/Makefile
1278–1279 ↗(On Diff #43855)

Makes sense. I'm betting most of SRCS_MIW is not need for lld. Perhaps only these 3 or only the last. Testing that...

-SRCS_EXT+=     DebugInfo/Symbolize/SymbolizableObjectFile.cpp
-SRCS_EXT+=     DebugInfo/Symbolize/Symbolize.cpp
+SRCS_MIW+=     DebugInfo/Symbolize/SymbolizableObjectFile.cpp
+SRCS_MIW+=     DebugInfo/Symbolize/Symbolize.cpp
...
-SRCS_EXT+=     Object/SymbolSize.cpp
+SRCS_MIW+=     Object/SymbolSize.cpp
lib/clang/libllvm/Makefile
1278–1279 ↗(On Diff #43855)

Yeah lld needs some of the large dwarf list. I am going to split that change out into a separate review.

lib/clang/libllvm/Makefile
1278–1279 ↗(On Diff #43855)

Remove MK_LLD_IS_LD check and move fix to D15915

This revision was not accepted when it landed; it landed in state Needs Review.Jun 20 2018, 4:10 PM
This revision was automatically updated to reflect the committed changes.