[PATCH] [Core] Update references in parallel
Rui Ueyama
ruiu at google.com
Tue Mar 17 14:06:52 PDT 2015
On Tue, Mar 17, 2015 at 1:58 PM, Sean Silva <chisophugis at gmail.com> wrote:
> REPOSITORY
> rL LLVM
>
> ================
> Comment at: lib/Core/Resolver.cpp:355
> @@ -353,3 +354,3 @@
> if (_symbolTable.isCoalescedAway(atom))
> _deadAtoms.insert(ref->target());
> continue;
> ----------------
> davide wrote:
> > ruiu wrote:
> > > I think this change is not thread-safe because of this line.
> _deadAtoms.insert is not thread-safe.
> > Oh this is really a bummer, considering it's such an hot codepath :(
> > Do you think it makes sense to surround insert with a lock and measure
> again and/or do you have a proposal for an alternative?
> Another solution would be to accumulate a separate vector in each thread,
> then merge them at the end. The `const_cast<Reference
> *>(ref)->setTarget(newTarget);` also needs some scrutiny. I would suggest
> testing these parallelism changes with threadsanitizer.
>
You suggested creating map<thread::id, vector<T>> to accumulate results? An
issue would be that before executing parallel_for_each, we don't know which
and how many threads are going to execute its body. That means we need to
extend the map in an atomic way.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150317/5eb7138f/attachment.html>
More information about the llvm-commits
mailing list