[PATCH] D65848: [ThinLTO][AutoFDO] Fix memory corruption due to race condition from thin backends

[Wenlei He via Phabricator via llvm-commits]

wenlei added a comment.

In D65848#1621227 <https://reviews.llvm.org/D65848#1621227>, @wmi wrote:

> Thanks for working on the issue.
>
> From reading the comment of LLVM_THREAD_LOCAL in include/llvm/Support/Compiler.h, it seems we should only use TLS for POD if we want it to be functioning as we expect on all platforms.
>
> How about change GUIDToFuncNameMap and CurrentModule as member fields in class GUIDToFuncNameMapper so we can create them separately for each module? Add a unique_ptr of GUIDToFuncNameMapper in SampleProfileLoader and a pointer field of type "GUIDToFuncNameMapper *" for each FunctionSamples object, so FunctionSamples object can access GUIDToFuncNameMap and CurrentModule via the pointer of GUIDToFuncNameMapper. The initialization of GUIDToFuncNameMapper pointer of each FunctionSamples object can be done in GUIDToFuncNameMapper's constructor (need to pass Reader as a param to get all the FunctionSamples).


Thanks for review. Using thread_local is the "minimum" fix, I actually implemented something similar to what you suggested (under testing), except I let GUIDToFuncNameMap be a member of SampleProfileLoader, and keep a reference in GUIDToFuncNameMapper. That way we can avoid TLS overhead. I will update once testing is done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65848