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

Wei Mi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 23 18:03:53 PST 2021


wmi added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:638
+                        SampleProfileReaderItaniumRemapper *Remapper,
+                        const StringMap<Function *> *SymbolMap = nullptr) const;
 
----------------
hoy wrote:
> wmi wrote:
> > hoy wrote:
> > > How about making this a static field of `FunctionSamples`?
> > That will look clearer, but I am not sure whether it will introduce data race problem. Look at the old problem Wenlei fixed: https://reviews.llvm.org/rG4b99b58a847c8424d9992a1e92a9f8ae7e4d7b51. The existing static members are of simple types and are all initialized with the same value for all the modules so they may be fine. SymbolMap read/write could be prone to such problem. 
> I actually meant to make the SymbolMap pointer as a static field. Does that sound safe?
Each module will have its own SymbolMap so the thread for one module may see other module's SymbolMap pointer. 

I think the other static vars in FunctionSamples can get away with the problem only because they are all initialized to the same value for different modules using the same profile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96932



More information about the llvm-commits mailing list