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

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

Apparently I didn't hit send...



================
Comment at: lld/COFF/LTO.cpp:210
+      objBuf = buf[i];
+    if (objBuf.empty())
       continue;
----------------
MaskRay wrote:
> When can `objBuf` be empty?
This is meant to preserve existing behavior from the original split loops. In the case where neither `buf` nor `files` contains an object, no input file was added. I assume that this can happen if there was an error during code generation.

---

Also, it occurs to me that split loops from before were reordering the native object files for the final link, which seems like asking non-determinism: if the object is found in the cache, it gets added later. This could be a serious cache bug fix...


================
Comment at: lld/test/COFF/pdb-thinlto.ll:31
+
+; CHECK:                           Modules                           
+; CHECK: ============================================================
----------------
MaskRay wrote:
> Delete trailing spaces (invisible on Phabricator)
Speaking of oddities in the dump output, I wish it wouldn't print trailing spaces. =P


================
Comment at: lld/test/COFF/pdb-thinlto.ll:33
+; CHECK: ============================================================
+; CHECK: Mod 0000 | `{{.*}}main.exe.lto.obj`: 
+; CHECK: Obj: `{{.*}}main.exe.lto.obj`: 
----------------
MaskRay wrote:
> 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`?
I think the extra spaces in the real output are a bug, I would prefer to see the output as I have it here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78221