resolve comments

Author: mingmingl-llvm

llvm/include/llvm/ProfileData/SampleProf.h

@@ -63,7 +63,6 @@ enum class sampleprof_error {
   zlib_unavailable,
   hash_mismatch,
   illegal_line_offset,
-  duplicate_vtable_type
 };
 
 inline std::error_code make_error_code(sampleprof_error E) {
@@ -1023,14 +1022,17 @@ class FunctionSamples {
     return VirtualCallsiteTypeCounts;
   }
 
-  /// Returns the vtable access samples for the C++ types at the un-drifted
-  /// location of \p Loc.
+  /// Returns the vtable access samples for the C++ types for \p Loc.
+  /// Under the hood, the caller-specified \p Loc will be un-drifted before the
+  /// type sample lookup if possible.
   TypeCountMap &getTypeSamplesAt(const LineLocation &Loc) {
     return VirtualCallsiteTypeCounts[mapIRLocToProfileLoc(Loc)];
   }
 
   /// Scale \p Other sample counts by \p Weight and add the scaled result to the
-  /// type samples for the undrifted location of \p Loc.
+  /// type samples for \p Loc. Under the hoold, the caller-provided \p Loc will
+  /// be un-drifted before the type sample lookup if possible.
+  /// typename T is either a std::map or a DenseMap.
   template <typename T>
   sampleprof_error addCallsiteVTableTypeProfAt(const LineLocation &Loc,
                                                const T &Other,

llvm/lib/ProfileData/SampleProf.cpp

@@ -111,8 +111,6 @@ class SampleProfErrorCategoryType : public std::error_category {
       return "Function hash mismatch";
     case sampleprof_error::illegal_line_offset:
       return "Illegal line offset in sample profile data";
-    case sampleprof_error::duplicate_vtable_type:
-      return "Duplicate vtable type in one map";
     }
     llvm_unreachable("A value of sampleprof_error has no message.");
   }

llvm/lib/ProfileData/SampleProfReader.cpp

@@ -200,6 +200,8 @@ enum class LineType {
   VirtualCallTypeProfile,
 };
 
+// Parse `Input` as a white-space separated list of `vtable:count` pairs. An
+// example input line is `_ZTVbar:1471 _ZTVfoo:630`.
 static bool parseTypeCountMap(StringRef Input,
                               DenseMap<StringRef, uint64_t> &TypeCountMap) {
   for (size_t Index = Input.find_first_not_of(' '); Index != StringRef::npos;) {
@@ -312,7 +314,6 @@ static bool ParseLine(const StringRef &Input, LineType &LineTy, uint32_t &Depth,
         n4 = AfterColon.find_first_of(' ');
         n4 = (n4 != StringRef::npos) ? n3 + n4 + 1 : Rest.size();
         StringRef WordAfterColon = Rest.substr(n3 + 1, n4 - n3 - 1);
-        // Break the loop if parsing integer succeeded.
         if (!WordAfterColon.getAsInteger(10, count))
           break;
 
@@ -642,17 +643,25 @@ SampleProfileReaderBinary::readVTableTypeCountMap(TypeCountMap &M) {
     auto VTableSamples = readNumber<uint64_t>();
     if (std::error_code EC = VTableSamples.getError())
       return EC;
-
-    if (!M.insert(std::make_pair(*VTableType, *VTableSamples)).second)
-      return sampleprof_error::duplicate_vtable_type;
+    // The source profile should not have duplicate vtable records at the same
+    // location. In case duplicate vtables are found, reader can emit a warning
+    // but continue processing the profile.
+    if (!M.insert(std::make_pair(*VTableType, *VTableSamples)).second) {
+      Ctx.diagnose(DiagnosticInfoSampleProfile(
+          Buffer->getBufferIdentifier(), 0,
+          "Duplicate vtable type " + VTableType->str() +
+              " at the same location. Additional counters will be ignored.",
+          DS_Warning));
+      continue;
+    }
   }
   return sampleprof_error::success;
 }
 
 std::error_code
 SampleProfileReaderBinary::readCallsiteVTableProf(FunctionSamples &FProfile) {
-  if (!ReadVTableProf)
-    return sampleprof_error::success;
+  assert(ReadVTableProf &&
+         "Cannot read vtable profiles if ReadVTableProf is false");
 
   // Read the vtable type profile for the callsite.
   auto NumCallsites = readNumber<uint32_t>();
@@ -761,7 +770,10 @@ SampleProfileReaderBinary::readProfile(FunctionSamples &FProfile) {
       return EC;
   }
 
-  return readCallsiteVTableProf(FProfile);
+  if (ReadVTableProf)
+    return readCallsiteVTableProf(FProfile);
+
+  return sampleprof_error::success;
 }
 
 std::error_code

llvm/lib/ProfileData/SampleProfWriter.cpp

@@ -855,8 +855,9 @@ std::error_code SampleProfileWriterExtBinaryBase::writeHeader(
 
 std::error_code SampleProfileWriterBinary::writeCallsiteVTableProf(
     const CallsiteTypeMap &CallsiteTypeMap, raw_ostream &OS) {
-  if (!WriteVTableProf)
-    return sampleprof_error::success;
+  assert(WriteVTableProf &&
+         "writeCallsiteVTableProf should not be called if WriteVTableProf is "
+         "false");
 
   encodeULEB128(CallsiteTypeMap.size(), OS);
   for (const auto &[Loc, TypeMap] : CallsiteTypeMap) {
@@ -912,7 +913,10 @@ std::error_code SampleProfileWriterBinary::writeBody(const FunctionSamples &S) {
         return EC;
     }
 
-  return writeCallsiteVTableProf(S.getCallsiteTypeCounts(), OS);
+  if (WriteVTableProf)
+    return writeCallsiteVTableProf(S.getCallsiteTypeCounts(), OS);
+
+  return sampleprof_error::success;
 }
 
 /// Write samples of a top-level function to a binary file.