[PATCH] D96932: [SampleFDO] Support enabling -funique-internal-linkage-name

Tue Feb 23 20:46:44 PST 2021

wmi added inline comments.

================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:741
   void findInlinedFunctions(DenseSet<GlobalValue::GUID> &S, const Module *M,
+                            const StringMap<Function *> &SymbolMap,
                             uint64_t Threshold) const {
----------------
hoy wrote:
> wmi wrote:
> > hoy wrote:
> > > Is `SymbolMap` always available to this function?  Looks like it could be null for other functions like `findFunctionSamplesAt`.
> > Yes, the function is used to collect function GUID with profile but not present in current module for ThinLTO importing. That function is only used in SampleLoaderPass so SymbolMap is always available. findFunctionSamplesAt has been used more widely than just SampleLoaderPass so SymbolMap is not always available. 
> As far as I understand, `SymbolMap` is always available in compiler but not tools. Does that mean tools should never do canonicalization?  Sorry, forgot to ask this earlier.
Right, like we discussed here: https://reviews.llvm.org/D96932#2580530, I agree it is better to let compiler do all the canonicalization work. 

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96932/new/

https://reviews.llvm.org/D96932