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

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added inline comments.


================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1104
 
-  auto AddStream = [&](size_t Task) {
+  auto AddStream = [&](size_t Task, Twine File) {
     return std::make_unique<CachedFileStream>(std::move(OS),
----------------
tejohnson wrote:
> I think you might need to mark the new parameter as unused here and in other cases where it isn't used via LLVM_ATTRIBUTE_UNUSED, to avoid build warnings - I can't recall how strictly that is enforced.
LLVM_ATTRIBUTE_UNUSED is used to suppress unused functions. For unused parameter, I don't see any build warnings when compiling with this patch. I feel like it's very common to have unused parameter. 


================
Comment at: lld/COFF/LTO.cpp:229
+    StringRef ltoObjName;
+    if (bitcodeFilePath == "ld-temp.o") {
+      ltoObjName =
----------------
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?


================
Comment at: lld/COFF/LTO.cpp:245
+      saveBuffer(buf[i].second, ltoObjName);
     ret.push_back(make<ObjFile>(ctx, MemoryBufferRef(objBuf, ltoObjName)));
   }
----------------
tejohnson wrote:
> The above changes affect both the MemoryBufferRef name as well as the saveTemps output file name. I assume the change to the former is what is required for PDB, is that correct?
Yes, it changes both the MemoryBufferRef and the saveTemps output file name otherwise the saveTemps output file name won't match with the the names in pdb. The former is what's written into PDB.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137217