[PATCH] D80406: [LLD][ThinLTO] A switch to allow compilation of only one module.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Mostly looks good, and I think this feature will be useful. Some test suggestions.

The subject should be more specific. It can be like: Add --thinlto-single-module to allow compiling partial modules.



================
Comment at: lld/test/ELF/lto/thinlto-single-module.ll:12
+; RUN: ld.lld main.o thin.a --thinlto-single-module=main.o --lto-obj-path=single1.o
+; RUN: llvm-readelf -S -s single1.o | FileCheck %s -check-prefix=DEFAULT
+; RUN: llvm-readelf -S -s single1.o1 | FileCheck %s -check-prefix=MAIN
----------------
Prefer `--check-prefix` to `-check-prefix`


================
Comment at: lld/test/ELF/lto/thinlto-single-module.ll:14
+; RUN: llvm-readelf -S -s single1.o1 | FileCheck %s -check-prefix=MAIN
+; RUN: not ls single1.o2
+;
----------------
Add a `not ls a.out` to check the output is suppressed


================
Comment at: lld/test/ELF/lto/thinlto-single-module.ll:22
+; RUN: not ls single1.o3
+;
+;; --thinlto-single-module=thin1.o should result in only thin1.o compiled.
----------------
No need to use `;` on empty lines.


================
Comment at: lld/test/ELF/lto/thinlto-single-module.ll:24
+;; --thinlto-single-module=thin1.o should result in only thin1.o compiled.
+; RUN: ld.lld main.o thin.a --thinlto-single-module=thin1.o --lto-obj-path=single3.o
+; RUN: llvm-readelf -S -s single3.o | FileCheck %s -check-prefix=DEFAULT
----------------
thin1.o/single3.o is a redundant case. It can be deleted.


================
Comment at: lld/test/ELF/lto/thinlto-single-module.ll:41
+; RUN: ls main.o.thinlto.bc
+; RUN: not ls thin.*.thinlto.bc
+
----------------
Move `IDX` lines here.

```
; FileCheck %s --check-prefix=IDX < single5.idx
; count 1 < single5.idx

; IDX: main.o
```


================
Comment at: lld/test/ELF/lto/thinlto-single-module.ll:43
+
+;; Check temporay output generated for main.o only.
+; RUN: ld.lld main.o thin.a --thinlto-single-module=main.o --save-temps
----------------
typo: temporary


================
Comment at: lld/test/ELF/lto/thinlto-single-module.ll:60
+
+; DEFAULT:       Value        Size Type Bind   Vis     Ndx Name
+; DEFAULT:   0000000000000000    0 FILE LOCAL  DEFAULT ABS ld-temp.o
----------------
Some reviewers prefer CHECK lines closer to the associated RUN lines. (I am one of them:))

i.e.

```
RUN: ...

DEFAULT:
MAIN:

RUN: ...

FOO: ...
BLAH: ...
```


================
Comment at: lld/test/ELF/lto/thinlto-single-module.ll:63
+
+; MAIN:           Value        Size Type Bind   Vis     Ndx Name
+; MAIN:      0000000000000000    0 FILE LOCAL  DEFAULT ABS thinlto-single-module.ll
----------------
superfluous space on this line


================
Comment at: lld/test/ELF/lto/thinlto-single-module.ll:71
+
+; BLAH:           Value        Size Type Bind   Vis     Ndx Name
+; BLAH:      0000000000000000    0 FILE LOCAL  DEFAULT ABS thin2.ll
----------------
superfluous space on this line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80406