Improve error reporting for mismatched value sites in IPGO

[dmikulin]

Summary

The error message for mismatched value sites is very cryptic.
This is an attempt to make it more readable.

[probinson]

LGTM. The previous diagnostic would be useful only to compiler developers.

compiler-rt/lib/profile/InstrProfData.inc

 VALUE_PROF_FUNC_PARAM(uint64_t, LargeValue, Type::getInt64Ty(Ctx))
 #endif /*VALUE_RANGE_PROF */
 #undef VALUE_PROF_FUNC_PARAM
 #undef INSTR_PROF_COMMA
 /* VALUE_PROF_FUNC_PARAM end */
 
 /* VALUE_PROF_KIND start */
 #ifndef VALUE_PROF_KIND
-#define VALUE_PROF_KIND(Enumerator, Value)
+#define VALUE_PROF_KIND(Enumerator, Value, Descr)
 #else
 #define INSTR_PROF_DATA_DEFINED
 #endif
 /* For indirect function call value profiling, the addresses of the target
  * functions are profiled by the instrumented code. The target addresses are
  * written in the raw profile data and converted to target function name's MD5
  * hash by the profile reader during deserialization.  Typically, this happens
  * when the raw profile data is read during profile merging.
  *
  * For this remapping the ProfData is used.  ProfData contains both the function
  * name hash and the function address.
  */
-VALUE_PROF_KIND(IPVK_IndirectCallTarget, 0)
+VALUE_PROF_KIND(IPVK_IndirectCallTarget, 0, "indirect call target")
 /* For memory intrinsic functions size profiling. */
-VALUE_PROF_KIND(IPVK_MemOPSize, 1)
+VALUE_PROF_KIND(IPVK_MemOPSize, 1, "memory intrinsic functions size")
 /* These two kinds must be the last to be
  * declared. This is to make sure the string
  * array created with the template can be
  * indexed with the kind value.
  */
-VALUE_PROF_KIND(IPVK_First, IPVK_IndirectCallTarget)
-VALUE_PROF_KIND(IPVK_Last, IPVK_MemOPSize)
+VALUE_PROF_KIND(IPVK_First, IPVK_IndirectCallTarget, "first")
+VALUE_PROF_KIND(IPVK_Last, IPVK_MemOPSize, "last")
 
 #undef VALUE_PROF_KIND
 /* VALUE_PROF_KIND end */
 
 /* COVMAP_FUNC_RECORD start */
 /* Definition of member fields of the function record structure in coverage
  * map.
  */

compiler-rt/lib/profile/InstrProfiling.h

 #define PROFILE_INSTRPROFILING_H_
 
 #include "InstrProfilingPort.h"
 
 #define INSTR_PROF_VISIBILITY COMPILER_RT_VISIBILITY
 #include "InstrProfData.inc"
 
 enum ValueKind {
-#define VALUE_PROF_KIND(Enumerator, Value) Enumerator = Value,
+#define VALUE_PROF_KIND(Enumerator, Value, Descr) Enumerator = Value,
 #include "InstrProfData.inc"
 };
 
 typedef void *IntPtrT;
 typedef struct COMPILER_RT_ALIGNAS(INSTR_PROF_DATA_ALIGNMENT)
     __llvm_profile_data {
 #define INSTR_PROF_DATA(Type, LLVMType, Name, Initializer) Type Name;
 #include "InstrProfData.inc"

llvm/include/llvm/ProfileData/InstrProf.h

 bool isIRPGOFlagSet(const Module *M);
 
 /// Check if we can safely rename this Comdat function. Instances of the same
 /// comdat function may have different control flows thus can not share the
 /// same counter variable.
 bool canRenameComdatFunc(const Function &F, bool CheckAddressTaken = false);
 
 enum InstrProfValueKind : uint32_t {
-#define VALUE_PROF_KIND(Enumerator, Value) Enumerator = Value,
+#define VALUE_PROF_KIND(Enumerator, Value, Descr) Enumerator = Value,
 #include "llvm/ProfileData/InstrProfData.inc"
 };
 
 /// Get the value profile data for value site \p SiteIdx from \p InstrProfR
 /// and annotate the instruction \p Inst with the value profile meta data.
 /// Annotate up to \p MaxMDCount (default 3) number of records per value site.
 void annotateValueSite(Module &M, Instruction &Inst,
                        const InstrProfRecord &InstrProfR,

llvm/include/llvm/ProfileData/InstrProfData.inc

 VALUE_PROF_FUNC_PARAM(uint64_t, LargeValue, Type::getInt64Ty(Ctx))
 #endif /*VALUE_RANGE_PROF */
 #undef VALUE_PROF_FUNC_PARAM
 #undef INSTR_PROF_COMMA
 /* VALUE_PROF_FUNC_PARAM end */
 
 /* VALUE_PROF_KIND start */
 #ifndef VALUE_PROF_KIND
-#define VALUE_PROF_KIND(Enumerator, Value)
+#define VALUE_PROF_KIND(Enumerator, Value, Descr)
 #else
 #define INSTR_PROF_DATA_DEFINED
 #endif
 /* For indirect function call value profiling, the addresses of the target
  * functions are profiled by the instrumented code. The target addresses are
  * written in the raw profile data and converted to target function name's MD5
  * hash by the profile reader during deserialization.  Typically, this happens
  * when the raw profile data is read during profile merging.
  *
  * For this remapping the ProfData is used.  ProfData contains both the function
  * name hash and the function address.
  */
-VALUE_PROF_KIND(IPVK_IndirectCallTarget, 0)
+VALUE_PROF_KIND(IPVK_IndirectCallTarget, 0, "indirect call target")
 /* For memory intrinsic functions size profiling. */
-VALUE_PROF_KIND(IPVK_MemOPSize, 1)
+VALUE_PROF_KIND(IPVK_MemOPSize, 1, "memory intrinsic functions size")
 /* These two kinds must be the last to be
  * declared. This is to make sure the string
  * array created with the template can be
  * indexed with the kind value.
  */
-VALUE_PROF_KIND(IPVK_First, IPVK_IndirectCallTarget)
-VALUE_PROF_KIND(IPVK_Last, IPVK_MemOPSize)
+VALUE_PROF_KIND(IPVK_First, IPVK_IndirectCallTarget, "first")
+VALUE_PROF_KIND(IPVK_Last, IPVK_MemOPSize, "last")
 
 #undef VALUE_PROF_KIND
 /* VALUE_PROF_KIND end */
 
 /* COVMAP_FUNC_RECORD start */
 /* Definition of member fields of the function record structure in coverage
  * map.
  */

llvm/lib/ProfileData/InstrProfWriter.cpp

std::unique_ptr<MemoryBuffer> InstrProfWriter::writeBuffer() {
   ProfOStream POS(OS);
   // Write the hash table.
   writeImpl(POS);
   // Return this in an aligned memory buffer.
   return MemoryBuffer::getMemBufferCopy(Data);
 }
 
 static const char *ValueProfKindStr[] = {
-#define VALUE_PROF_KIND(Enumerator, Value) #Enumerator,
+#define VALUE_PROF_KIND(Enumerator, Value, Descr) #Enumerator,
 #include "llvm/ProfileData/InstrProfData.inc"
 };
 
 void InstrProfWriter::writeRecordInText(StringRef Name, uint64_t Hash,
                                         const InstrProfRecord &Func,
                                         InstrProfSymtab &Symtab,
                                         raw_fd_ostream &OS) {
   OS << Name << "\n";

llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp

void PGOUseFunc::annotateValueSites() {
 
   // Create the PGOFuncName meta data.
   createPGOFuncNameMetadata(F, FuncInfo.FuncName);
 
   for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind)
     annotateValueSites(Kind);
 }
 
+static const char *ValueProfKindDescr[] = {
+#define VALUE_PROF_KIND(Enumerator, Value, Descr) Descr,
+#include "llvm/ProfileData/InstrProfData.inc"
+};
+
 // Annotate the instructions for a specific value kind.
 void PGOUseFunc::annotateValueSites(uint32_t Kind) {
+  assert(Kind <= IPVK_Last);
   unsigned ValueSiteIndex = 0;
   auto &ValueSites = FuncInfo.ValueSites[Kind];
   unsigned NumValueSites = ProfileRecord.getNumValueSites(Kind);
   if (NumValueSites != ValueSites.size()) {
     auto &Ctx = M->getContext();
     Ctx.diagnose(DiagnosticInfoPGOProfile(
         M->getName().data(),
-        Twine("Inconsistent number of value sites for kind = ") + Twine(Kind) +
-            " in " + F.getName().str(),
+        Twine("Inconsistent number of value sites for ") +
+            Twine(ValueProfKindDescr[Kind]) +
+            Twine(" profiling in \"") + F.getName().str() +
+            Twine("\", possibly due to the use of a stale profile."),
         DS_Warning));
     return;
   }
 
   for (auto &I : ValueSites) {
     LLVM_DEBUG(dbgs() << "Read one value site profile (kind = " << Kind
                       << "): Index = " << ValueSiteIndex << " out of "
                       << NumValueSites << "\n");

llvm/test/Transforms/PGOProfile/Inputs/diag_no_value_sites.proftext

+# :ir is the flag to indicate this is IR level profile.
+:ir
+foo
[phone]
+1
+1

llvm/test/Transforms/PGOProfile/diag_no_value_sites.ll

+; RUN: llvm-profdata merge %S/Inputs/diag_no_value_sites.proftext -o %t.profdata
+; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s
+
+; CHECK: Inconsistent number of value sites for memory intrinsic functions size profiling
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @foo(i8* %dst, i8* %src, i64 %n) {
+entry:
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %n, i1 false)
+  ret void  
+}
+
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i1)