[llvm-commits] [PATCH] unit tests for MCJIT

[Jim Grosbach]

On Sep 25, 2012, at 4:53 PM, "Kaylor, Andrew" <andrew.kaylor at intel.com> wrote:

> Hi Jim,
> 
> I understand your point about layering, but I don't understand what the downside to providing a simple reference implementation is.  The fact that Dan proposed this as part of a group of new tests may make it seem as though he just wanted to share the lli code and so putting it in the public interface was an easy solution, but I actually think this is a useful piece of code in its own right.

I have no conceptual objection to having a reference implementation. That's orthogonal to the purpose of this patch, however, and should be considered and implemented separately. That's an important bit of code if we're making it a general purpose thing available for broader use, and should get lots of attention.

> 
> Specifically, I think it makes sense for all the pieces necessary to implement a basic MCJIT client to be available somewhere in the standard LLVM library set.  I don't see why the steps for writing a trivial MCJIT client should include "copy this class from lli."

Right. That's just a temporary growing pains thing for the MCJIT. echristo and I chatted with James Malloy about this a bit yesterday and today. Does any of that conversation help clarify where I'm coming from a bit?

> 
> Looking forward, I imagine there being several reference implementations of the memory manager interface available to accomplish various purposes.  For instance, there could be a TrivialMemoryManager (the one we're talking about), a SecureMemoryManager (which has code to set page protection flags after an object is loaded) and a RemoteMemoryManager (which handles the case of injecting JITed objects into a remote process).
> 

Yes, absolutely. I completely agree.

> It may be that advanced users would still want to provide their own implementation, but it wouldn't be necessary just to get something working.
> 

Yep. It'd only be the truly weird scenarios that would need it.

> -Andy
> 
> 
> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Jim Grosbach
> Sent: Monday, September 24, 2012 2:52 PM
> To: Du Toit, Stefanus
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [PATCH] unit tests for MCJIT
> 
> 
> On Sep 24, 2012, at 2:20 PM, "Du Toit, Stefanus" <stefanus.du.toit at intel.com> wrote:
> 
>>> -----Original Message-----
>>> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits- 
>>> bounces at cs.uiuc.edu] On Behalf Of Jim Grosbach
>>> Sent: Monday, September 24, 2012 1:38 PM
>>> To: Malea, Daniel
>>> Cc: llvm-commits at cs.uiuc.edu
>>> Subject: Re: [llvm-commits] [PATCH] unit tests for MCJIT
>>> 
>>> The most substantive comment is that we shouldn't move the memory 
>>> manager out of lli.cpp. That's very much not a general purpose memory 
>>> manager, but rather is intended as a (trivial) example of what a 
>>> client application may need to implement in order to utilize the 
>>> MCJIT. It's not part of a general LLVM library, but rather part of the lli tool.
>> 
>> What's the downside of moving the memory manager out of lli? If it's sufficient for a simple client (like lli's) needs, why not have it be reusable?
> 
>> My understanding is that the tests do need *some* memory manager implementation. If it's not moved here, wouldn't it need to be included from inside of the lli sources (awkward) or duplicated somewhere in the test sources (even more awkward)?
> 
> It's a division of responsibility and layering thing. The MCJIT library defines the interface to the memory manager, but that's it. The client is responsible for supplying an implementation. We want the implementation to make this distinction very starkly at least for now, and moving the memory manager into the library rather than part of lli obfuscates that purpose. It's possible we'll create another library later that supplies, for example, a simple default hosted JIT environment complete with simple memory manager and refactor lli to reference that. That's an interesting thing and I'm open to it as a design direction, but it's orthogonal to getting unit tests for other parts of the MCJIT and I'd prefer it be dealt with separately.
> 
> -Jim
> 
>> 
>> Thanks,
>> 
>> Stefanus
>> 
>> --
>> Stefanus Du Toit stefanus.du.toit at intel.com  Intel Dynamic Mobility 
>> and Parallelism  Intel Waterloo
>> phone: +1 519 772 2576
>> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits