[PATCH] D32023: [ProfileData] Support coverage for PE binaries

[Vedant Kumar via Phabricator via llvm-commits]

vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

LGTM with a few minor changes.



================
Comment at: lib/ProfileData/InstrProf.cpp:172
+std::string getInstrProfSectionName(const Module *M, InstrProfSectKind Kind) {
+  bool AddSegment = (M && Triple(M->getTargetTriple()).isOSBinFormatMachO());
+  std::string SectName;
----------------
We can simplify this function with an early return: `if (!M) return InstrProfSectName[Kind];`. One less null check.


================
Comment at: lib/Transforms/Instrumentation/ThreadSanitizer.cpp:284
       if (SectionName.endswith(getInstrProfCountersSectionName(
-            /*AddSegment=*/false)))
+            nullptr)))
         return false;
----------------
nit, clang-format


================
Comment at: tools/llvm-cov/TestingSupport.cpp:54
     if (Section.getName(Name))
       return 1;
+    if (Name == llvm::getInstrProfNameSectionName()) {
----------------
Could you replicate your TODO comment here? ( 'to support Win->ELF cross compilation with coverage properly'...)


https://reviews.llvm.org/D32023