llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.92k stars 11.52k forks source link

Make -thinlto-index-only work well with thin static archives #42062

Open rnk opened 5 years ago

rnk commented 5 years ago
Bugzilla Link 42717
Version unspecified
OS Windows NT
CC @smithp35

Extended Description

Many build systems (such as LLVM's) are organized as collections of static archives. Currently, the recommended way to use -thinlto-index-only to distribute LTO backend actions is to use --start-lib / --end-lib on the command line: http://lists.llvm.org/pipermail/llvm-dev/2019-June/133145.html

To ease adoption of thinlto in legacy build systems, LLD should at least handle thin archives, if not true static archives. There is a discussion of some of these issues and how to solve them here: https://reviews.llvm.org/D64461#1586526

It boils down to coming up with a good path for the .thinlto.bc file calculated here: http://llvm-cs.pcc.me.uk/lib/LTO/LTO.cpp#1206

With a very Chromium / LLVM perspective on things, I think the ideal index file name would be something like:

${pathtoobj}/${object_noext}.o # stage1 output ${pathtoobj}/${object_noext}.${binary_noext}.thinlto.bc # index file ${pathtoobj}/${object_noext}.${binary_noext}.imports # import list ${pathtoobj}/${object_noext}.${binary_noext}.o # native output

So, for chromium on Windows, we'd have:

obj/v8/v8_libbase/bits.obj obj/v8/v8_libbase/bits.d8.thinlto.bc obj/v8/v8_libbase/bits.d8.o obj/v8/v8_libbase/bits.chrome_child.thinlto.bc obj/v8/v8_libbase/bits.chrome_child.o ... one per thinlto binary

I (think?) today the current behavior is that every link will attempt to write obj/v8/v8_libbase/bits.thinlto.bc, which won't work if the same object is compiled into two binaries.

Amy expressed interest in working on this.

llvmbot commented 5 years ago

I attached a small script that exercises the functionality we are looking for. It generates two executables that both use a static library, but require different functions from the library (so if they were to use the same paths for writing the indices, they would be wrong). The script is annotated with "BUG" in places where there is work to be done. In particular:

(1) Not needing to copy files that are included in the thin archive.

(2) Needing a way to write build files that accounts for not all members in a static archive being used in every link.

llvmbot commented 5 years ago

test script that uses distributed ThinLTO with static archives

llvmbot commented 5 years ago

W.r.t. the naming of the outputs:

We already have one mechanism for creating index files and native object files with unique paths: -thinlto-prefix-replace: can be used to replace the start of the path where these would be created. For example, without the flag, obj/foo.obj would get index file obj/foo.obj.thinlto.bc. With -thinlto-prefix-replace:obj;lto.d8, this would become lto.d8/foo.obj.thinlto.bc, instead.

The existing mechanism works to get unique names, and I would say we can get -thinlto-inxdex-only to work with thin archives without implementing a new naming scheme. Then, if we still want the new naming scheme, we can do that after.

rnk commented 5 years ago

assigned to @amykhuang