[PATCH] D135014: [lld][ELF] Fix lazy ThinLTO index writing in thin archives

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 3 11:50:37 PDT 2022


MaskRay added a comment.

`thinlto-emit-index-thin-archive.ll` may be better since we already have `thinlto-emit-index.ll` and generally it's useful to make similar tests share a common prefix.

`thinlto-thin-archive-collision.ll` may be merged into the test as well.



================
Comment at: lld/test/ELF/lto/thinlto-index-files-thin-archive.ll:1
+;; Make sure that when we are linking against a thin archive in a different
+;; folder from the object files it references, bitcode files that have
----------------
The comment may be too long. I'll suggest

For a thin archive referencing object files in a different directory, emit index files (`lib.a($member at $offset).thinlto.bc`) in the directory containing the archive.


================
Comment at: lld/test/ELF/lto/thinlto-index-files-thin-archive.ll:10
+; RUN: llvm-as %p/Inputs/thinlto.ll -o ./test-folder-1/b.o
+; RUN: mkdir test-folder-2
+; RUN: llvm-ar -crT ./test-folder-2/lib.a test-folder-1/a.o test-folder-1/b.o
----------------
`%t` is about tests. `test-` in `test-folder-2` isn't useful. Just use `folder-1` `folder-2`, but actually `dir1` dir2` is more common.


================
Comment at: lld/test/ELF/lto/thinlto-index-files-thin-archive.ll:13
+; RUN: ld.lld --thinlto-emit-index-files -shared ./test-folder-2/lib.a -o c
+; RUN: ls ./test-folder-2 | FileCheck %s
+
----------------
Add another test using `--whole-archive lib.a`. The patch makes the behavior similar to `--whole-archive`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135014



More information about the llvm-commits mailing list