Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

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

Open Quuxplusone opened 5 years ago

Quuxplusone commented 5 years ago
Bugzilla Link PR42717
Status NEW
Importance P enhancement
Reported by Reid Kleckner (rnk@google.com)
Reported on 2019-07-22 15:10:30 -0700
Last modified on 2019-08-26 18:07:22 -0700
Version unspecified
Hardware PC Windows NT
CC llvm-bugs@lists.llvm.org, llvm@inglorion.net, smithp352@googlemail.com
Fixed by commit(s)
Attachments test.sh (3188 bytes, text/plain)
Blocks
Blocked by
See also
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.
Quuxplusone 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.

Quuxplusone commented 5 years ago

Attached test.sh (3188 bytes, text/plain): test script that uses distributed ThinLTO with static archives

Quuxplusone 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.