[PATCH] D66767: Add binary filename to the bitcode filename when using -thinlto-index-only

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 27 13:22:11 PDT 2019


rnk added a comment.

If I understand the prefix replace option correctly, it operates on the entire input bitcode file path. With your example of `-thinlto-prefix-replace:;test.exe`, that would end up creating a new tree of `thinlto.bc` files (and import files) in a new `test.exe` directory. While that works, it doesn't seem very easy for a build system to use. For myself as a compiler developer, I'd also prefer not to have to navigate between two parallel deeply nested directory trees to find intermediate inputs and outputs. So, I'd really like to come up with a solution that puts the `thinlto.bc` index file in the same directory as the input bitcode file. If we had a prefix replace variant that worked on the file basename instead of the full path, that seems like it would work well, and fit into the existing flow with the addition of one boolean flag.



================
Comment at: lld/COFF/InputFiles.cpp:796
+  // the object is compiled into multiple binaries.
+  StringRef binaryName = sys::path::filename(config->outputFile);
+  StringRef name =
----------------
This seems like an awkward place to introduce the binary name. This code is coming up with a fake "identifier" for the memory buffer that points into a static archive. I think we want to inject the binary name in the code that computes the output file path, which is in `lto::getThinLTOOutputFile`. I'm not sure what's the best way to thread the binary name through to that code, but I think that's the best place to do it.

Separately, I believe this code path only runs for non-thin archives, which aren't used for Chromium. There are two call sites for this ctor in Driver.cpp, and I think they pass an empty archive name for thin archives. Coming up with a unique name for regular, non-thin static archives is challenging, since there could even be multiple members with the same name, which is why offsetInArchive is factored in. I think we should keep that out of scope for now, and just focus on thin archives.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66767





More information about the llvm-commits mailing list