[PATCH] D144596: Add extra parameter to thinlto-prefix-replace

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D144596#4147936 <https://reviews.llvm.org/D144596#4147936>, @itf wrote:

> In D144596#4146593 <https://reviews.llvm.org/D144596#4146593>, @MaskRay wrote:
>
>> What's the motivation of this change?
>
> This is necessary for Bazel to support thinlto with tree artifacts.
>
> Bazel tree artifacts are generated folders with an unknown number of files, for example, auto generating source files from a script or the result of extracting a .zip file containing source files.
>
> Bazel tree artifacts require that the folders are "write once". Therefore we cannot insert the link objects in the same folder as the result from the thinlto indexing step. Currently the file  "--thinlto-index-only=file" requires the link objects to be in the same folder as the  summary+import files from the indexing step.
>
> This change allows having a different prefix for the link objects, and therefore keep the folders containing the summary+import files be "write once".

Could you give an example in the summary how an end user is going to use the proposed feature?
To describe how distributed ThinLTO works, I typically use such an example including ELF relocatable object files and LLVM bitcode files.

  echo 'int bb(); int main() { return bb(); }' > a.c
  echo 'int elf0(); int bb() { return elf0(); }' > b.c
  echo 'int cc() { return 0; }' > c.c
  echo 'int elf0() { return 0; }' > elf0.c && clang -c elf0.c
  echo '' > elf1.c && clang -c elf1.c
  
  clang -c -O2 -flto=thin a.c b.c c.c
  clang -flto=thin -fuse-ld=lld -Wl,--thinlto-index-only=a.rsp -Wl,--thinlto-prefix-replace=';lto/' elf0.o a.o -Wl,--start-lib b.o c.o -Wl,--end-lib elf1.o
  clang -c -O2 -fthinlto-index=lto/a.o.thinlto.bc a.o -o lto/a.o
  clang -c -O2 -fthinlto-index=lto/b.o.thinlto.bc b.o -o lto/b.o
  clang -c -O2 -fthinlto-index=lto/c.o.thinlto.bc c.o -o lto/c.o
  clang -fuse-ld=lld @a.rsp elf0.o elf1.o  # a.rsp contains lto/a.o and lto/b.o

IIUC the proposed third component of `;` just changes the `--thinlto-index-only=a.rsp` specified response file (not anything else on the disk), then I am unsure how it is going to help Bazel...
Bazel can preprocess the `a.rsp` content itself, instead of requiring more from the linker.

BTW: if you or someone is working on distributed ThinLTO in Bazel, I wonder whether you may investigate D130229 <https://reviews.llvm.org/D130229> to address some issues with the current `--thinlto-index-only=` design...



================
Comment at: lld/ELF/Driver.cpp:1260
+    error("--thinlto-prefix-replace=old_dir;new_dir;obj_dir only supported"
+          " when thinlto-index-only specifies an output file");
+  }
----------------
The idiomatic way is `must be used with --thinlto-index-only=`.


================
Comment at: lld/MachO/Driver.cpp:1585
+           config->thinLTOIndexFileObjectPrefixReplace) =
+      getOldNewOptionsExtra(args, OPT_thinlto_prefix_replace_eq);
   if (config->thinLTOEmitIndexFiles && !config->thinLTOIndexOnly) {
----------------
This has a build error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144596