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

[Wei Mi via Phabricator via llvm-commits]

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