tejohnson added a comment.
The naming convention used here for COFF is actually very similar to what is
done for other ThinLTO save-temps output files, which are placed at the input
module locations. We may want to just do this across the board. @MaskRay wdyt?
A few other questions/comments below.
================
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),
----------------
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.
================
Comment at: lld/COFF/LTO.cpp:229
+ StringRef ltoObjName;
+ if (bitcodeFilePath == "ld-temp.o") {
+ ltoObjName =
----------------
This case should always be i==0 I think?
================
Comment at: lld/COFF/LTO.cpp:245
+ saveBuffer(buf[i].second, ltoObjName);
ret.push_back(make<ObjFile>(ctx, MemoryBufferRef(objBuf, ltoObjName)));
}
----------------
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?
================
Comment at: lld/MachO/LTO.cpp:132
+ "ThinLTO", "Thin", config->thinLTOCacheDir,
+ [&](size_t task, Twine originFile, std::unique_ptr<MemoryBuffer> mb) {
+ files[task] = std::move(mb);
----------------
In some files this parameter is called "filed" and in others "originFile".
Better to be consistent, and perhaps to use a more descriptive name, something
like moduleName ?
================
Comment at: llvm/include/llvm/Support/Caching.h:53
///
/// if (AddStreamFn AddStream = Cache(Task, Key))
/// ProduceContent(AddStream);
----------------
This and possibly other header file descriptions need updates.
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
