[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