zequanwu added inline comments.

================
Comment at: lld/COFF/LTO.cpp:238
+      sys::path::append(path, directory,
+                        outputFileBaseName + ".lto." + baseName);
+      sys::path::remove_dots(path, true);
----------------
tejohnson wrote:
> MaskRay wrote:
> > What if two input bitcode files have the same basename, e.g. `dir1/a.obj` 
> > and `dir2/a.obj`?
> I think that should be ok as the output file path created here includes the 
> directory. So you should get dir1/a.out.lto.a.obj and dir2/a.out.lto.a.obj.
Yes, you will get dir1/a.out.lto.a.obj and dir2/a.out.lto.a.obj.


================
Comment at: lld/COFF/LTO.cpp:229
+    StringRef ltoObjName;
+    if (bitcodeFilePath == "ld-temp.o") {
+      ltoObjName =
----------------
tejohnson wrote:
> MaskRay wrote:
> > MaskRay wrote:
> > > 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.
> > I guess `if (i < config->ltoPartitions)` may fix the issue.
> Ah ok, forgot about the lto partitions case. Sorry, @zequanwu, looks like you 
> will need to go back to your old handling that appends i if it is non-zero.
I reverted this part back. Since input bitcode file can be called `ld-temp.o`, 
i could be any number smaller than maxTasks, removed assertion. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137217

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to