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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

I find the summary difficult to follow and I believe users will be more confusing than me (as a ThinLTO contributor).

The new semantics can be used this way by newer Bazel: `-Wl,--thinlto-index-only=bazel-bin/$package/b-lto-final.params,--thinlto-emit-imports-files,--thinlto-prefix-replace=bazel-bin/;bazel-bin/$package/b.lto/;bazel-bin/$package/b.lto.backend/,--lto-obj-path=bazel-bin/$package/b.lto.merged.o`

- `bazel-bin/$package/_objs/b/b.pic.o`: bitcode file
- `bazel-bin/$package/_objs.indexing/b/b.pic.o`: minimized bitcode file
- `bazel-bin/$package/b.lto/$package/_objs/b/b.pic.o.thinlto.bc`: index file
- `bazel-bin/$package/b.lto/$package/_objs/b/b.pic.o.imports`: import file
- `bazel-bin/$package/b.lto.backend/$package/_objs/b/b.pic.o`: ELF relocatable object file after a backend compile
- `bazel-bin/$package/b-lto-final.params`: response file for the final native link (it references ELF relocatable object files after backend compiles)

There are two advantages:

- the full bitcode file and the minimized bitcode file are in different directories. This allows a ThinLTO indexing action to ship fewer input files. This doesn't require lld changes.
- The ELF relocatable object file after a backend compile is now in a different directory. This ensures the directory `b.lto` is only used by one action. This requires this lld patch.

AIUI `--thinlto-object-suffix-replace=.indexing.o;.o` can effectively be deprecated. So after a transition period when we finally remove `--thinlto-object-suffix-replace`, the feature as proposed in this patch will not increase the overall complexity.

Therefore, I am happy with the direction.



================
Comment at: lld/test/ELF/lto/thinlto-index-file-object-prefix-replace.ll:5
+; RUN: opt -module-summary %s -o oldpath/prefix_replace/1.o
+; RUN: opt -module-summary %p/Inputs/thinlto.ll -o oldpath/prefix_replace/2.o
+; RUN: opt -module-summary %p/Inputs/thinlto_empty.ll -o oldpath/3.o
----------------
Perhaps shorten directories like `oldpath`, `newpath`, `prefix_replace` to something like `old, new, out`.


================
Comment at: lld/test/ELF/lto/thinlto-index-file-object-prefix-replace.ll:8
+
+;; Ensure lld writes linked files to linked objects file
+; RUN: ld.lld --thinlto-index-only=1.txt -shared oldpath/prefix_replace/1.o oldpath/prefix_replace/2.o oldpath/3.o -o /dev/null
----------------
End full sentences in comments with periods. Fix this everywhere.


================
Comment at: lld/test/ELF/lto/thinlto-index-file-object-prefix-replace.ll:53
+}
\ No newline at end of file

----------------
Fix "No newline at end of file"


================
Comment at: lld/test/MachO/thinlto-index-file-object-prefix-replace.ll:3
+; RUN: rm -rf %t ; split-file %s %t && cd %t
+; RUN: mkdir -p oldpath oldpath/prefix_replace
+
----------------
Just one positional argument `mkdir -p oldpath/prefix_replace`


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

https://reviews.llvm.org/D144596