vsk added a comment.
This looks great, that turned out to be a lot cleaner than I expected.
================
Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1717
+ FilenameStrs[0] = getCurrentDirname();
+ FilenameRefs[0] = FilenameStrs[0];
for (const auto &Entry : FileEntries) {
----------------
There's some nice alignment here with your change that gets rid of
DecompressedData. If CoverageFilenamesSectionWriter were to accept an
ArrayRef<string>, this strange FilenameRefs container can go away.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5302
+ // Add in -fcoverage-compilation-dir if necessary.
+ if (!Triple.isNVPTX() && !Triple.isAMDGCN())
+ addCoverageCompDirArg(Args, CmdArgs, D.getVFS());
----------------
Perhaps leave a comment explaining why the compdir flag differs for these GPU
targets?
================
Comment at: llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h:178
private:
- std::vector<StringRef> Filenames;
+ std::vector<std::string> Filenames;
std::vector<ProfileMappingRecord> MappingRecords;
----------------
This is a nice cleanup that should probably happen before/independently of the
format change.
================
Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp:149
+ StringRef CWD;
+ if (auto Err = readString(CWD))
+ return Err;
----------------
This would need to be gated on the CovMapVersion. For context, the format needs
to maintain backwards compatibility so that newer IDE's can inspect binaries
built by older compilers.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95753/new/
https://reviews.llvm.org/D95753
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
