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

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:632
+  /// If \p SymbolMap is not nullptr, findFunctionSamplesAt will do
+  /// canonication for \p CalleeName. If \p SymbolMap is not provided, user
+  /// has to make sure \p CalleeName has already been canonicalized or has no
----------------
Nit: canonicalization? 


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:638
+                        SampleProfileReaderItaniumRemapper *Remapper,
+                        const StringMap<Function *> *SymbolMap = nullptr) const;
 
----------------
How about making this a static field of `FunctionSamples`?


================
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 {
----------------
Is `SymbolMap` always available to this function?  Looks like it could be null for other functions like `findFunctionSamplesAt`.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1747
   }
+  assert(SymbolMap.count("") == 0 &&
+         "No empty StringRef should be added in SymbolMap");
----------------
Nit: "" -> StringRef() ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96932