[PATCH] [Core] Update references in parallel
ruiu at google.com
Tue Mar 17 14:20:56 PDT 2015
On Tue, Mar 17, 2015 at 2:13 PM, Sean Silva <chisophugis at gmail.com> wrote:
> On Tue, Mar 17, 2015 at 2:06 PM, Rui Ueyama <ruiu at google.com> wrote:
>> On Tue, Mar 17, 2015 at 1:58 PM, Sean Silva <chisophugis at gmail.com>
>>> rL LLVM
>>> Comment at: lib/Core/Resolver.cpp:355
>>> @@ -353,3 +354,3 @@
>>> if (_symbolTable.isCoalescedAway(atom))
>>> 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.
> This sounds like a deficiency in the API of parallel_for_each. Maybe it
> could take a 4th argument lambda that is called with the number of threads
> or an array of thread::id's?
lld:parallel_for_each is designed after Microsoft's
concurrency::parallel_for_each. These functions signatures are actually the
same. If we change it, we need to implement our own even for Windows
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits