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

[Nicolas Geoffray]

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
>