[PATCH] D70048: [LLD] Add NetBSD support as a new flavor of LLD (nb.lld)

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 2 13:15:12 PST 2019


MaskRay added a comment.

> If we could get this patch merged, we could build and link the whole LLVM with lld on NetBSD and it would increase the productivity of the bot (better build times). Right now we need to maintain hacks to link at most with 2/3 cores, while 5/6 ones are idling doing nothing due to enormous RAM consumption of GNU ld.

I will be very happy to see the productivity of NetBSD build bot gets improved. My comments below are about the approach you choose in this patch.

> FreeBSD/OpenBSD ship with mutations of the behavior of ELF/GNU/Linux in certain nits, we do the same with our driver.

The FreeBSD specific code is just the following 4 lines in ELF/Driver.cpp:

  if (s.endswith("_fbsd")) {
    s = s.drop_back(5);
    osabi = ELFOSABI_FREEBSD;
  }

`PT_OPENBSD_*` extensions are related to https://bugs.llvm.org//show_bug.cgi?id=31288 , which predated my involvement into lld.
It was not a good example of issue reports. Neither the bug nor the link it referenced provided rationale why such extensions were needed. CC @bradsmith here. I will also ask in their IRC channel and/or send an email to their mailing list.

The NetBSD extension does much more than FreeBSD (4 lines of code) and OpenBSD (7 lines of code modulo comments).
On the contrary, the NetBSD patch introduces another toplevel driver nb.lld, and includes ~200 lines of change.
Moreoever, I anticipate you will post another patch with probably than more than 200 lines of code that does https://blog.netbsd.org/tnf/entry/the_first_report_on_lld#handling-of-indirect-shared-library-dependencies , which many people (probably even the GNU maintainers) feel no longer appropriate nowadays.

> the linker MUST work in the standalone mode

By standalone mode, do you mean $(LD) or ld? I think for most such use cases, we should probably change them to do `gcc/clang -nostdlib` instead. In most cases users are not supposed to call ld directly. On FreeBSD (and Google servers and Android, if you consider them distributions), it has been proved many packages don't need ld customization to function properly.

If you can't migrate off those packages right now, a shell script that gets installed at "/usr/bin/ld" will probably work.

> clang NetBSD driver shall not hardcode LLD specific options

Several toolchains in the clang/lib/Driver/ToolChains detect lld and make lld specific decisions there. While I hope linker specific options should be kept at a minimum, I understand that sometimes it is unavoidable. When we have no choice but to add linker specific options, adding such code to clangDriver is an established practice followed by almost every toolchain (see Android, Fuchsia, MinGW, MSVC, etc). Doing the driver thing in two places will more likely to cause conflicts.

> the linker must have support for cross-building

I think this patch will actually harm cross building that targets NetBSD. Before, ld.lld is shared by all ELF platforms: Linux, FreeBSD, Fuchsia, Android, ChromeOS, etc. If an ELF change works on Linux, it will assuredly work on other platforms. This patch will introduce a new driver that naturally gets less test coverage.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70048/new/

https://reviews.llvm.org/D70048





More information about the llvm-commits mailing list