[PATCH] D78221: [COFF] Assign unique identifiers to ObjFiles from LTO

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

> Use the unique filenames that are used when /lldsavetemps is passed.

Is this for debug convenience? (`llvm-pdbutil dump` output) Or required by some tools?



================
Comment at: lld/COFF/LTO.cpp:210
+      objBuf = buf[i];
+    if (objBuf.empty())
       continue;
----------------
When can `objBuf` be empty?


================
Comment at: lld/test/COFF/pdb-thinlto.ll:2
+; REQUIRES: x86
+; RUN: rm -fr %T/thinlto
+; RUN: mkdir %T/thinlto
----------------
`%T` is deprecated

https://llvm.org/docs/CommandGuide/lit.html

You can use `%t.dir` or just `%t`

```
; RUN: rm -rf %t.dir
; RUN: mkdir %t.dir
; RUN: cd %t.dir
```

Then you can remove one `rm -Rf` below


================
Comment at: lld/test/COFF/pdb-thinlto.ll:14
+
+; Run again with the cache. Make sure we get the same object names.
+
----------------
Not sure whether COFF wants to adopt this convention: in lld/ELF and some binary utilities, we start to use `## ` (translated to .ll: `;; `) for comments, `# ` (`; `) for RUN/CHECK lines.

This convention makes comments slightly easier to be discovered.


================
Comment at: lld/test/COFF/pdb-thinlto.ll:31
+
+; CHECK:                           Modules                           
+; CHECK: ============================================================
----------------
Delete trailing spaces (invisible on Phabricator)


================
Comment at: lld/test/COFF/pdb-thinlto.ll:33
+; CHECK: ============================================================
+; CHECK: Mod 0000 | `{{.*}}main.exe.lto.obj`: 
+; CHECK: Obj: `{{.*}}main.exe.lto.obj`: 
----------------
You may indent `Mod 0000` by 2 spaces to reflect the actual output... but why does llvm-pdbutil print two spaces only for `Mod 0000`?


================
Comment at: lld/test/COFF/weak-external.test:8
 
-# CHECK: lto.tmp
-# CHECK-NEXT: lto.tmp
+# CHECK: .lto.obj:
+# CHECK-NEXT: .lto.obj:
----------------
Nit: add 5 spaces to make CHECK1 content aligned with the next line.


================
Comment at: lld/test/COFF/weak-external3.test:9
 
-# CHECK1: lto.tmp
-# CHECK1: lto.tmp
+# CHECK1: .lto.obj:
+# CHECK1-NEXT: .lto.obj:
----------------
Nit: add 5 spaces to make `CHECK1` content aligned with the next line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78221