[PATCH] [Core] Update references in parallel

Rui Ueyama 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>
>> 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.
>>
>
> 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
platform.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150317/f59de504/attachment.html>


More information about the llvm-commits mailing list