[llvm-dev] LLVM multithreading support

[Mehdi AMINI via llvm-dev]

On Sun, Apr 12, 2020 at 1:17 PM Chris Lattner <clattner at nondot.org> wrote:

>
>
> On Apr 12, 2020, at 11:27 AM, Mehdi AMINI <joker.eph at gmail.com> wrote:
>
> Hey Chris,
>
>
> On Sat, Apr 11, 2020 at 5:15 PM Chris Lattner via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Hi all,
>>
>> I was looking at the profile for a tool I’m working on, and noticed that
>> it is spending 10% of its time doing locking related stuff.  The structure
>> of the tool is that it reading in a ton of stuff (e.g. one moderate example
>> I’m working with is 40M of input) into MLIR, then uses its multithreaded
>> pass manager to do transformations.
>>
>> As it happens, the structure of this is that the parsing pass is single
>> threaded, because it is parsing through a linear file (the parser is simple
>> and fast, so this is bound by IR construction).  This means that none of
>> the locking during IR construction is useful.
>>
>
> I'm curious which are the places that show up on the profile? Do you have
> a few stacktraces to share?
>
>
> In my case, it is all MLIR attribute/type uniquification stuff which is
> guarded by a RWMutex.
>
> Historically, LLVM had a design where you could dynamically enable and
>> disable multithreading support in a tool, which would be perfect for this
>> use case, but it got removed by this patch
>> <https://github.com/llvm/llvm-project/commit/9c9710eaf4c1f01b8b518bdba89aba059ab14175#diff-bb772a7e6d4ebf2b46c6d42c95f40ddf>:
>> (xref https://reviews.llvm.org/D4216).  The rationale in the patch
>> doesn’t make sense to me - this mode had nothing to do with the old LLVM
>> global lock, this had to do with whether llvm::llvm_is_multithreaded()
>> returned true or false … which all the locking stuff is guarded on.
>>
>
> It seems that at the time the assumption was that this flag was there to
> alleviate the cost of the global lock only and removing the lock removed
> the motivation for the feature? Looks like you proved this wrong :)
>
> +Zach, David, and Reid to make sure they don't miss this.
>
>
> Yeah, it was about not paying the cost for synchronization when it wasn’t
> worthwhile.
>
> Would it make sense to re-enable this, or am I missing something?
>>
>
> Finding a way to re-enable it seems interesting. I wonder how much it'll
> interact with the places inside the compiler that are threaded now, maybe
> it isn't much more than tracking and auditing the uses of
> LLVM_ENABLE_THREADS (like lib/Support/ThreadPool.cpp for example). Have you
> already looked into it?
>
>
> It is super-easy to reenable, because the entire codebase is still
> calling llvm::llvm_is_multithreaded().  We just need to add the global
> back, along with the methods to set and clear the global, and
> change llvm::llvm_is_multithreaded() to something like:
>
> bool llvm::llvm_is_multithreaded() {
> #if LLVM_ENABLE_THREADS != 0
> return someGlobal;
> #else
> return false;
> #endif
> }
>
>
This is the part I am not sure about: the ThreadPool I mentioned above for
example is not checking llvm_is_multithreaded() I believe, and I doubt that
the client of the ThreadPool are. So you can have multiple things in flight
in the ThreadPool that relies on llvm::Mutex and similar things to operate
properly.
Seems like we could end-up in a situation where llvm_is_multithreaded() is
returning false, effectively disabling all the mutex and other safety,
while some code would use the ThreadPool.

-- 
Mehdi
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200412/47756dba/attachment.html>