[PGO] Comment how function pointers for indirect calls are mapped to function names

[anemet]

Summary

Hopefully this will make it easier for the next person to figure all
this out...

[davidxl]

lgtm

include/llvm/ProfileData/InstrProfData.inc

76

After this is committed, please update the copy of this file in compiler-rt as well.

[anemet]

include/llvm/ProfileData/InstrProfData.inc

76

Done in r264612.

[davidxl]

This should be updated again with the version using C style comment.

David

[anemet]

In D18490#384802, @davidxl wrote:

This should be updated again with the version using C style comment.

Done in r264615.

[MaskRay]

Is

llvm/trunk/include/llvm/ProfileData/InstrProfData.inc

78 ↗

(On Diff #51814)

Is FunctionPointer still used by the runtime? @davidxl @vsk

I can only find one reference __llvm_get_function_addr, which is a public API.

[MaskRay]

llvm/trunk/include/llvm/ProfileData/InstrProfData.inc

78 ↗

(On Diff #51814)

Hmm. It is used by ProfileData/InstrProfReader.cpp

So it seems that value profiling adds quite a bit size overhead to the metadata sections.

[davidxl]

llvm/trunk/include/llvm/ProfileData/InstrProfData.inc

78 ↗

(On Diff #51814)

Right -- it is used by the profile reader to build mapping from func->name so that indirect call target profile can be translated to func names. Changing the llvm-profdata workflow by forcing binary as an input will allow the field to be removed -- but things get complicated with -fPIE build.

include/llvm/ProfileData/InstrProfData.inc

INSTR_PROF_DATA(const uint64_t, llvm::Type::getInt64Ty(Ctx), NameRef, \
                 ConstantInt::get(llvm::Type::getInt64Ty(Ctx), \
     IndexedInstrProf::ComputeHash(getPGOFuncNameVarInitializer(Inc->getName()))))
 INSTR_PROF_DATA(const uint64_t, llvm::Type::getInt64Ty(Ctx), FuncHash, \
                 ConstantInt::get(llvm::Type::getInt64Ty(Ctx), \
                 Inc->getHash()->getZExtValue()))
 INSTR_PROF_DATA(const IntPtrT, llvm::Type::getInt64PtrTy(Ctx), CounterPtr, \
                 ConstantExpr::getBitCast(CounterPtr, \
                 llvm::Type::getInt64PtrTy(Ctx)))
+// This is used to map function pointers for the indirect call targets to
+// function name hashes during the conversion from raw to merged profile data.
 INSTR_PROF_DATA(const IntPtrT, llvm::Type::getInt8PtrTy(Ctx), FunctionPointer, \
                 FunctionAddr)
 INSTR_PROF_DATA(IntPtrT, llvm::Type::getInt8PtrTy(Ctx), Values, \
                 ConstantPointerNull::get(Int8PtrTy))
 INSTR_PROF_DATA(const uint32_t, llvm::Type::getInt32Ty(Ctx), NumCounters, \
                 ConstantInt::get(llvm::Type::getInt32Ty(Ctx), NumCounters))
 INSTR_PROF_DATA(const uint16_t, Int16ArrayTy, NumValueSites[IPVK_Last+1], \
                 ConstantArray::get(Int16ArrayTy, Int16ArrayVals))

lib/ProfileData/InstrProfReader.cpp

RawInstrProfReader<IntPtrT>::readValueProfilingData(InstrProfRecord &Record) {
   ErrorOr<std::unique_ptr<ValueProfData>> VDataPtrOrErr =
       ValueProfData::getValueProfData(ValueDataStart,
                                       (const unsigned char *)ProfileEnd,
                                       getDataEndianness());
 
   if (VDataPtrOrErr.getError())
     return VDataPtrOrErr.getError();
 
+  // Note that besides deserialization, this also performs the conversion for
+  // indirect call targets.  The function pointers from the raw profile are
+  // remapped into function name hashes.
   VDataPtrOrErr.get()->deserializeTo(Record, &Symtab->getAddrHashMap());
   CurValueDataSize = VDataPtrOrErr.get()->getSize();
   return success();
 }
 
 template <class IntPtrT>
 std::error_code
 RawInstrProfReader<IntPtrT>::readNextRecord(InstrProfRecord &Record) {