[PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D137217#3926601 <https://reviews.llvm.org/D137217#3926601>, @tejohnson wrote:

> @MaskRay wondering if this is a good change to make for ELF as well, wdyt?

Yes, I think this is a good idea and improves debuggability. The change is non-trivial so so this patch focusing on the COFF part is good.



================
Comment at: lld/COFF/LTO.cpp:238
+      sys::path::append(path, directory,
+                        outputFileBaseName + ".lto." + baseName);
+      sys::path::remove_dots(path, true);
----------------
What if two input bitcode files have the same basename, e.g. `dir1/a.obj` and `dir2/a.obj`?


================
Comment at: lld/COFF/LTO.cpp:229
+    StringRef ltoObjName;
+    if (bitcodeFilePath == "ld-temp.o") {
+      ltoObjName =
----------------
tejohnson wrote:
> zequanwu wrote:
> > tejohnson wrote:
> > > This case should always be i==0 I think?
> > IIUC, "ld-temp.o" is the name of combined module. Do you mean there will be only 1 combined module and it will always be the first task?
> Yes. So you don't need the handling for i==0 in the name in this case (you could probably assert that i==0).
This looks like a hack. `assert(i == 0)` will fail with `-opt:lldltopartitions=2`: `buf[1]` will be called `ld-temp.o` as well.

In addition, if an input bitcode file is called `ld-temp.o` (for some reason they don't use `.obj`, just `ld-temp.o`), `assert(i == 0)` will fail as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137217