[llvm-commits] [llvm] r57340 - /llvm/trunk/lib/ExecutionEngine/JIT/JIT.cpp

[Nicolas Geoffray]

Hi Mon,

Thanks for reverting!

Indeed, we do not point out that the default module provider does not 
lock by default. I made a patch some time ago, but Chris thinks the 
locking should be made at a higher level:

http://lists.cs.uiuc.edu/pipermail/llvmdev/2008-April/013947.html



Mon Ping Wang wrote:
> Out of curiosity, does the Java compiler grab the JIT lock for its own uses?
>   

Yes it does. What's happening is that the Java compiler will call a 
classloader.loadClass function (I assume you're familiar with Java?) 
that will return a class with a bunch of Java functions. The compiler 
then locates the function in the class, takes the JIT lock and creates 
the IR.

Note that I also take the JIT lock for materializing IR :), but there is 
no deadlock anymore since it's done at the very last step.

Nicolas

>    -- Mon Ping
>
>
>
> On Oct 10, 2008, at 10:09 AM, Nicolas Geoffray wrote:
>
>   
>> Hi Mon,
>>
>> The logic here is that materializing a function has to be guarded
>> because it creates LLVM IR, constants and types. However, it is not  
>> the
>> JIT's responsability to guard it, but your module provider  
>> implementation.
>>
>> A default implementation of the module provider only locates the the
>> function and creates the IR. So it's ok to let the JIT lock. However
>> (and this is what's happening in Java) we can imagine complex things
>> where materializing a function requires calling some external code.  
>> When
>> this complex module provider is able to create the IR after calling  
>> this
>> external code, *then* it can take the lock.
>>
>> I acknowledge that it's easier to place the lock in the execution
>> engine, but it's wrong :)
>>
>> Nicolas
>>
>> Mon Ping Wang wrote:
>>     
>>> Hi Nicolas,
>>>
>>> Sorry, I was not aware of Java issues.  I moved the locked upwards
>>> because if I didn't, I ran into a problem that materialized function
>>> got corrupted occasionally and I got some weird errors.  I'm not sure
>>> if this also could occur in Java as it is possible that it will never
>>> call getPointerToFunction twice on the same function by different
>>> threads.  Is the requirement that materializing the function must be
>>> lockless?  If no, maybe we can create a local lock around the
>>> materialization so we don't try to do it at the same time.   
>>> Otherwise,
>>> clients would need guard calls to getPointerToFunction.
>>>
>>>   -- Mon Ping
>>>
>>>
>>> On Oct 10, 2008, at 1:25 AM, Nicolas Geoffray wrote:
>>>
>>>
>>>       
>>>> Hi Mon,
>>>>
>>>> We're having a conflict here.
>>>>
>>>> Originally, materializing a function was guarded. However, in Java,
>>>> materializing a function may execute Java code that use locks, hence
>>>> deadlocks can happen.
>>>> So I moved the guard after materialization.
>>>>
>>>> On a more general note I *don't* think materializing a function  
>>>> should
>>>> be guarded by the JIT compiler. The JIT's mutex should only protect
>>>> JIT
>>>> internal data. Materializing a function does not modify any JIT  
>>>> data.
>>>>
>>>> Nicolas
>>>>
>>>>
>>>> Mon P Wang wrote:
>>>>
>>>>         
>>>>> Author: wangmp
>>>>> Date: Thu Oct  9 20:47:42 2008
>>>>> New Revision: 57340
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=57340&view=rev
>>>>> Log:
>>>>> Moved guard mutex upwards to guard materializing a function
>>>>> in getPointerToFunction
>>>>>
>>>>>
>>>>> Modified:
>>>>>   llvm/trunk/lib/ExecutionEngine/JIT/JIT.cpp
>>>>>
>>>>> Modified: llvm/trunk/lib/ExecutionEngine/JIT/JIT.cpp
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/JIT/JIT.cpp?rev=57340&r1=57339&r2=57340&view=diff
>>>>>
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> = 
>>>>> = 
>>>>> ===================================================================
>>>>> --- llvm/trunk/lib/ExecutionEngine/JIT/JIT.cpp (original)
>>>>> +++ llvm/trunk/lib/ExecutionEngine/JIT/JIT.cpp Thu Oct  9 20:47:42
>>>>> 2008
>>>>> @@ -489,6 +489,8 @@
>>>>>  if (void *Addr = getPointerToGlobalIfAvailable(F))
>>>>>    return Addr;   // Check if function already code gen'd
>>>>>
>>>>> +  MutexGuard locked(lock);
>>>>> +
>>>>>  // Make sure we read in the function if it exists in this Module.
>>>>>  if (F->hasNotBeenReadFromBitcode()) {
>>>>>    // Determine the module provider this function is provided by.
>>>>> @@ -509,13 +511,11 @@
>>>>>      abort();
>>>>>    }
>>>>>  }
>>>>> -
>>>>> +
>>>>>  if (void *Addr = getPointerToGlobalIfAvailable(F)) {
>>>>>    return Addr;
>>>>>  }
>>>>>
>>>>> -  MutexGuard locked(lock);
>>>>> -
>>>>>  if (F->isDeclaration()) {
>>>>>    void *Addr = getPointerToNamedFunction(F->getName());
>>>>>    addGlobalMapping(F, Addr);
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>
>>>>>
>>>>>           
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>>>         
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>>       
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>     
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>