[PATCH][llvm-c] Expose MC JIT

Sun Apr 21 19:14:01 PDT 2013

OK - I have a new patch for review, which incorporates Andrew's feedback.

The patch exposes the MCJIT via the C API.

The current API uses the SectionMemoryManager by default.  I plan to expose the ability to have the C API call back into the client's code, and use the client's own memory manager, in the future.  But even then, the default will be SectionMemoryManager.  Because this requires calling applyPermissions(), I also expose the ExecutionEngine::finalizeObject() method.  This was tricky - I take it that in the future, this method will take a Module*M parameter to specify which module to finalize.  In order to not create future confusion in the C API, I expose this as LLVMFinalizeAllObjects() and specify the API's semantics as being that all objects associated with the execution engine should be finalized.

The patch also exposes the NoFramePointerElim option.  The manner in which options are exposed is designed for forward compatibility; you supply an options struct along with a size which you zero-fill prior to manipulating.  This is similar to the idiom I've seen used in other C APIs like BerkeleyDB.  I considered having separate C function calls for each option, in the style of the ExecutionEngineBuilder API - but while that idiom feels right to me in C++, it feels less C-like.  As well, the current options approach exposes not just parts of the Builder but also part of TargetOptions (namely, NoFramePointerElim).  It's also more concise in practice.

I plan to expose more innards through the LLVMMCJITCompilerOptions in the future.  I'd be happy to do more of that in one go if that was preferred; but I thought that a baby step would be the best thing for now.

-Filip

On Apr 21, 2013, at 6:26 PM, Filip Pizlo <fpizlo at apple.com> wrote:

> 
> On Apr 15, 2013, at 10:42 AM, "Kaylor, Andrew" <andrew.kaylor at intel.com> wrote:
> 
>> OK, let me start by saying that MCJIT does take ownership of the memory manager.  It doesn’t use an OwningPtr, which would make this clear, but it does delete the pointer in its destructor.  I think this is happening because we needed some finer control over when the MM got deleted.  I should probably revisit this and at least add some comments to make it clear what’s happening and why.  It might not even be an issue anymore, because I did some work a while ago to try to clean up object ownership issues.
> 
> Actually, I was just confused.  MCJIT deletes MemMgr, which is always aliased to Dyld, as far as I can tell.  So I was just wrong. :-)
> 
>>  
>> That said, I have been meaning for some time to break apart the JIT and MCJIT interfaces.  The fact that they are both abstracted by ExecutionEngine and EngineBuilder complicates that, but it really needs to be done (as you are seeing).
>>  
>> For now, would it be possible to have the C-interface provide a wrapper that supplies empty implementations of the irrelevant functions when creating a memory manager for MCJIT?
> 
> I think this is sensible.  I will proceed in this way.
> 
> Thanks!
> 
>>  
>> -Andy
>>  
>> From: Filip Pizlo [mailto:fpizlo at apple.com] 
>> Sent: Saturday, April 13, 2013 1:18 AM
>> To: Kaylor, Andrew
>> Cc: llvm-commits at cs.uiuc.edu
>> Subject: Re: [PATCH][llvm-c] Expose MC JIT
>>  
>> Ah - good thing you pointed this out.  I just realized that my patch is wrong.  Perhaps I can get some feedback on the best way to architect this.
>>  
>> Here's the problem:
>>  
>> - MCJIT does not take ownership of the memory manager.  Hence allocating one in the constructor is wrong; it'll leak when MCJIT dies.  But deleting the memory manager passed to MCJIT would be a change in behavior, and I'm not sure if it's in line with either what existing users expect or what was intended.  Insofar as the JIT instance corresponds to ownership of modules, it feels like it shouldn't also take ownership of the memory manager; for example you might imagine wanting to throw away the MCJIT but keep the code it generated and continue to use the memory manager to track it - and eventually free it.  But EngineBuilder currently claims that the ExecutionEngine takes ownership of the JMM - I'm assuming that this is just wrong documentation, and that EngineBuilder's use of the same JMM option for both JIT and MCJIT is just not right.
>>  
>> - I'd like to expose SectionMemoryManager and, eventually in a separate patch, the ability to create custom RTDyldMemoryManagers via the C API.  I'd prefer this to be an RTDyldMemoryManager and not a JITMemoryManager, since the latter has a load of methods that are not relevant to MCJIT.  But EngineBuilder wants a JITMemoryManager.  This would mean that the C API would have to pass its RTDyldMemoryManager via a cast to JITMemoryManager just so MCJIT could then use it as an RTDyldMemoryManager again.  Seems wrong.  I'm assuming that the correct long-term thing is to fix the EngineBuilder to not pass the JMM to the MCJIT, since it's good to expose the fact that the MCJIT actually just wants an RTDyldMemoryManager instead.
>>  
>> In short, I'd like to have a separate EngineBuilder setting for the RTDyldMemoryManager.  If this is specified and you end up using the JIT and not MCJIT, you get an error.  If you use the MCJIT, then the RTDyldMemoryManager option overrides the JMM option.  Or something similar.
>>  
>> Does that make sense?
>>  
>> -Filip
>>  
>>  
>> On Apr 12, 2013, at 5:38 PM, Filip Pizlo <fpizlo at apple.com> wrote:
>> 
>> 
>> Thanks for the feedback!  I will try this change and see what happens. 
>> 
>> -Filip
>> 
>> On Apr 12, 2013, at 5:35 PM, "Kaylor, Andrew" <andrew.kaylor at intel.com> wrote:
>> 
>> Hi Filip,
>>  
>> I’ll take a closer look at your patches on Monday, but my initial input is that the default memory manager used should be SectionMemoryManager rather than the DefaultJITMemoryManager.
>>  
>> Thanks,
>> Andy
>>  
>> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Filip Pizlo
>> Sent: Friday, April 12, 2013 4:49 PM
>> To: llvm-commits at cs.uiuc.edu
>> Subject: Re: [PATCH][llvm-c] Expose MC JIT
>>  
>> Revised patches included.
>>  
>> I added additional ruggedizing to the LLVMCreateMCJITCompilerForModule function, so that if it detects that the passed struct is larger than expected, it reports an error instead of continuing.
>>  
>> _______________________________________________
>> 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130421/acccc394/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mcjit.patch
Type: application/octet-stream
Size: 6180 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130421/acccc394/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130421/acccc394/attachment-0001.html>