<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div></div><div><br></div><div>-Filip</div><div><br></div><br><div><div>On Apr 22, 2013, at 2:02 PM, Filip Pizlo <<a href="mailto:fpizlo@apple.com">fpizlo@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">OK - I think that calling finalizeObject() within the C API is a reasonable kludge for now. I'll have a patch shortly.<div><br></div><div>More broadly, getting MCJIT into a better shape is on my critical path, as well. See: <a href="https://bugs.webkit.org/show_bug.cgi?id=112840">https://bugs.webkit.org/show_bug.cgi?id=112840</a>. I'd like to help out with the de-sloppiness-ifying of it, so I'd like to understand better what the end goal is. I already know that I want to control memory management, cache management, and page permissions myself as a client, and preferably I want to accomplish this by supplying my own RuntimeDyld (or JITMemoryManager). I also know that I'll want to improve error tolerance - for example in case of memory allocation failure (honestly, I haven't investigated this much, so this may already be supported - I just don't see how through the API). So, I'm particularly interesting in seeing how to get the MCJIT into a great shape while supporting both the currently-envisioned use cases in LLVM and its other clients, and also the way that WebKit will use it! This should be fun. :-)</div><div><br></div><div>-Filip</div><div><br></div><div><br><div><div>On Apr 22, 2013, at 11:51 AM, "Kaylor, Andrew" <<a href="mailto:andrew.kaylor@intel.com">andrew.kaylor@intel.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div lang="EN-US" link="blue" vlink="purple" style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="WordSection1" style="page: WordSection1;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">I wasn’t sure the C API needed to support the case of JITing into another address space. I guess it makes sense that we’d want to make that available eventually.<o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">I do think it would be better to have MCJIT finalize the object from getPointerToFunction(), but as I said we can’t do that until there’s another way to trigger code generation. This is part of the sloppiness I referred to earlier. There is a web of interactions going on within MCJIT and there are some constraints on the order in which things can be done. Right now MCJIT has a patchwork way of handling this, but it really needs to be cleaned up to manage things in a more intentional way.<o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Anyway, my immediate goal here is to figure out the simplest reasonable thing we can do to get your patch working in a sustainable way.<o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">I don’t know if anything other than lli is relying on the current side-effects of MCJIT::getPointerToFunction(). I’d prefer to hold off on putting new things in the MCJIT interface until the aforementioned clean-up is done. I was suggesting putting the call to finalizeObject in the C API because I’d rather rush something into implementation than rush something into an interface, but depending on your timeline for making the C API I could try to bump the priority of the MCJIT state handling clean-up on my end.<o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">-Andy<o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span></div><div><div style="border-style: solid none none; border-top-color: rgb(181, 196, 223); border-top-width: 1pt; padding: 3pt 0in 0in;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><b><span style="font-size: 10pt; font-family: Tahoma, sans-serif;">From:</span></b><span style="font-size: 10pt; font-family: Tahoma, sans-serif;"><span class="Apple-converted-space"> </span>Filip Pizlo [<a href="mailto:fpizlo@apple.com">mailto:fpizlo@apple.com</a>]<span class="Apple-converted-space"> </span><br><b>Sent:</b><span class="Apple-converted-space"> </span>Monday, April 22, 2013 11:25 AM<br><b>To:</b><span class="Apple-converted-space"> </span>Kaylor, Andrew<br><b>Cc:</b><span class="Apple-converted-space"> </span>David Tweed; Eric Christopher;<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><b>Subject:</b><span class="Apple-converted-space"> </span>Re: [PATCH][llvm-c] Expose MC JIT<o:p></o:p></span></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><o:p> </o:p></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">Wouldn't it be better to just have MCJIT call finalizeObject in getPointerToFunction and friends, and eradicate ExecutionEngine::finalizeObject() completely?<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><o:p> </o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">OTOH, if I understand right and the goal of the MCJIT is to support the notion of JITing into someone else's address space, then finalizeObject() makes loads of sense: you want the client to be able to say when it happens. If that's true then it should be exposed to the C API. <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><br>-Filip<o:p></o:p></div></div><div><p class="MsoNormal" style="margin: 0in 0in 12pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><br>On Apr 22, 2013, at 10:57 AM, "Kaylor, Andrew" <<a href="mailto:andrew.kaylor@intel.com" style="color: purple; text-decoration: underline;">andrew.kaylor@intel.com</a>> wrote:<o:p></o:p></p></div><blockquote style="margin-top: 5pt; margin-bottom: 5pt;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">It definitely won’t be necessary to expose more things through the C API. At most, things will need to be added to the implementation.</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">I’m wondering if we can even avoid having to put the ‘finalize’ concept into the API.</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">As I understand it, the normal work flow would be something like this:</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span>1.<span style="font-style: normal; font-variant: normal; font-weight: normal; font-size: 7pt; line-height: normal; font-family: 'Times New Roman';"> <span class="Apple-converted-space"> </span></span></span><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Create a module and populate it</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span>2.<span style="font-style: normal; font-variant: normal; font-weight: normal; font-size: 7pt; line-height: normal; font-family: 'Times New Roman';"> <span class="Apple-converted-space"> </span></span></span><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Create an execution engine for the module</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span>3.<span style="font-style: normal; font-variant: normal; font-weight: normal; font-size: 7pt; line-height: normal; font-family: 'Times New Roman';"> <span class="Apple-converted-space"> </span></span></span><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Get a pointer to a function in the module</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span>4.<span style="font-style: normal; font-variant: normal; font-weight: normal; font-size: 7pt; line-height: normal; font-family: 'Times New Roman';"> <span class="Apple-converted-space"> </span></span></span><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Execute the function</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">If that’s right, I guess MCJIT trips because while its implementation of getPointerTo[Named]Function triggers compilation it doesn’t cause permissions to be applied or invalidate the code cache. This happens because MCJIT needs to handle the case where the generated code is going to be executed in another process (and possibly on another system), so it doesn’t make assumptions about when everything is in its final place. The C API, however, could arguably make such assumptions.</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">What this boils down to is that somewhere between step 2 above and step 4 above we need to:</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span>1.<span style="font-style: normal; font-variant: normal; font-weight: normal; font-size: 7pt; line-height: normal; font-family: 'Times New Roman';"> <span class="Apple-converted-space"> </span></span></span><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Generate the code</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span>2.<span style="font-style: normal; font-variant: normal; font-weight: normal; font-size: 7pt; line-height: normal; font-family: 'Times New Roman';"> <span class="Apple-converted-space"> </span></span></span><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Apply relocations</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span>3.<span style="font-style: normal; font-variant: normal; font-weight: normal; font-size: 7pt; line-height: normal; font-family: 'Times New Roman';"> <span class="Apple-converted-space"> </span></span></span><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Apply memory permissions</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span>4.<span style="font-style: normal; font-variant: normal; font-weight: normal; font-size: 7pt; line-height: normal; font-family: 'Times New Roman';"> <span class="Apple-converted-space"> </span></span></span><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Invalidate the code cache</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">MCJIT does 1 and 2, if necessary, in response to a getPointerToFunction() call. Arguably it could also do 3 and 4 there, since in the remote case the client code isn’t going to want pointers to functions. The trouble is that there are places (such as lli) where we are using that call to trigger code emission even though we may still want to move things around before 3 and 4 happen. That wouldn’t be a problem if we exposed a function to trigger code emission directly, but we don’t right now.</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">However, it may be reasonable for the C API implementation to call MCJIT::finalizeObject when its getPointerToFunction equivalent is called.</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">That leaves us with invalidating the code cache. I don’t see any reason that the memory manager shouldn’t do that automatically when the applyPermissions function is called. I notice that currently invalidateInstructionCache is part of the SectionMemoryManager interface but not the RuntimeDyldMemoryManager, so that’s a problem already if we don’t just make it part of applyPermissions().</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">So what I’m thinking is that if you can add a call to MCJIT::finalizeObject in the appropriate places in the C API implementation then the FinalizeAllObjects method can be removed completely. I’ll add an invalidateInstructionCache() call in the SectionMemoryManager::applyPermissions() implementation, and that should take care of the ARM issue.</span><o:p></o:p></div></blockquote><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><o:p> </o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">Sounds good. <o:p></o:p></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><br><br><o:p></o:p></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Does that sound reasonable?</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">-Andy</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div><div><div style="border-style: solid none none; border-top-color: rgb(181, 196, 223); border-top-width: 1pt; padding: 3pt 0in 0in;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><b><span style="font-size: 10pt; font-family: Tahoma, sans-serif;">From:</span></b><span style="font-size: 10pt; font-family: Tahoma, sans-serif;"><span class="Apple-converted-space"> </span>Filip Pizlo [<a href="mailto:fpizlo@apple.com" style="color: purple; text-decoration: underline;">mailto:fpizlo@apple.com</a>]<span class="Apple-converted-space"> </span><br><b>Sent:</b><span class="Apple-converted-space"> </span>Monday, April 22, 2013 10:19 AM<br><b>To:</b><span class="Apple-converted-space"> </span>Kaylor, Andrew<br><b>Cc:</b><span class="Apple-converted-space"> </span>David Tweed; Eric Christopher;<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;">llvm-commits@cs.uiuc.edu</a><br><b>Subject:</b><span class="Apple-converted-space"> </span>Re: [PATCH][llvm-c] Expose MC JIT</span><o:p></o:p></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"> <o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"> <o:p></o:p></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">On Apr 22, 2013, at 10:06 AM, "Kaylor, Andrew" <<a href="mailto:andrew.kaylor@intel.com" style="color: purple; text-decoration: underline;">andrew.kaylor@intel.com</a>> wrote:<o:p></o:p></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><br><br><br><o:p></o:p></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">The state management in MCJIT is quite sloppy right now. I agree that invalidating the code cache is an issue that needs to be considered. It seems to me that MCJIT itself ought to be able to do that when it needs to be done if it were paying attention. At the very least, it could happen in the 'finalizeObject' method. That's somewhat tangential to the patch at hand, but it is a consideration. If there's something quick I can do to MCJIT to make this work, that's probably preferable to pushing something into the C-interface implementation. I'll give that some thought today, but if anyone else is interested I'd be happy to make it a discussion rather than just a private rumination.<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">Hopefully we can do this without exposing more stuff via the C API. I think that finalizeObject() should do this, but I will think about it some more.<o:p></o:p></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><br><br><br><o:p></o:p></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><br>Otherwise, my main question about the patch has to do with the nature of the C-interface API. Is that API treated as a contract that needs to be respected from release to release or are we free to tinker with it as needed? The thing that worries me is how this interface will survive the transition to multiple module support.<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">I believe that this is the goal of the C API, yes. It is also a goal of this patch to be forward-compatible in this way.<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">My patch defends against this in two ways:<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">1) When creating the MCJIT via the C API, I just follow the same convention as other ExecutionEngines do: you specify a module, but you can add one later. Right now calling AddModule on an MCJIT instance will crash, and the documentation tells you this. Once the MCJIT supports multiple modules, I believe that this should Just Work - you will then be able to call AddModule.<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">2) I don't expose finalizeObject() directly. Instead I created a new API called FinalizeAllObjects(), which requires that all modules associated with the execution engine get finalized at the time of call. My understanding is that it is safe to finalizeObject() if you've already done it before - it's currently idempotent. So if MCJIT goes multi-module, then this API will still have a well-defined behavior, and this behavior will not be different from what my patch does.<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">-Filip<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"> <o:p></o:p></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><br><br><br><o:p></o:p></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><br>-Andy<br><br>-----Original Message-----<br>From:<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits-bounces@cs.uiuc.edu" style="color: purple; text-decoration: underline;">llvm-commits-bounces@cs.uiuc.edu</a><span class="Apple-converted-space"> </span>[<a href="mailto:llvm-commits-bounces@cs.uiuc.edu" style="color: purple; text-decoration: underline;">mailto:llvm-commits-bounces@cs.uiuc.edu</a>] On Behalf Of David Tweed<br>Sent: Monday, April 22, 2013 7:33 AM<br>To: 'Eric Christopher'; Filip Pizlo<br>Cc:<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;">llvm-commits@cs.uiuc.edu</a><br>Subject: RE: [PATCH][llvm-c] Expose MC JIT<br><br>Hi,<br><br>Not a comment on the general idea (which seems like a good one), but asking detail question: some architectures (such as ARM) invalidating cache entries at more times than, eg, x86. (IIRC ARM needs to invalidate the cache at the transition time between being "data" and "instructions".) It looks to me like this can be done in LLVMFinalizeAllObjects(), but I'm cc:ing someone much, much more knowledgeable about this than me... It's certainly desirable that the API provides enough points at which the MC JIT is in command that it can call all the (possibly platform specific) permissions/cache invalidation operations needed on that memory without user code needing to do it.<br><br>Other than that, the patch looks like a good patch to me (but again Andrew is the main authority).<br><br>Cheers,<br>Dave<br><br><br>-----Original Message-----<br>From:<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits-bounces@cs.uiuc.edu" style="color: purple; text-decoration: underline;">llvm-commits-bounces@cs.uiuc.edu</a><br>[<a href="mailto:llvm-commits-bounces@cs.uiuc.edu" style="color: purple; text-decoration: underline;">mailto:llvm-commits-bounces@cs.uiuc.edu</a>] On Behalf Of Eric Christopher<br>Sent: 22 April 2013 14:22<br>To: Filip Pizlo<br>Cc:<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;">llvm-commits@cs.uiuc.edu</a><br>Subject: Re: [PATCH][llvm-c] Expose MC JIT<br><br>The API and rationale seem totally reasonable to me. Up to Andy to approve though.<br><br>-eric<br><br>On Sun, Apr 21, 2013 at 7:14 PM, Filip Pizlo <<a href="mailto:fpizlo@apple.com" style="color: purple; text-decoration: underline;">fpizlo@apple.com</a>> wrote:<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">OK - I have a new patch for review, which incorporates Andrew's feedback.<br><br>The patch exposes the MCJIT via the C API.<br><br>The current API uses the SectionMemoryManager by default. I plan to<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">expose<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">the ability to have the C API call back into the client's code, and<span class="apple-converted-space"> </span><br>use<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">the<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">client's own memory manager, in the future. But even then, the<span class="apple-converted-space"> </span><br>default<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">will<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">be SectionMemoryManager. Because this requires calling<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">applyPermissions(),<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">I also expose the ExecutionEngine::finalizeObject() method. This was<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">tricky<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">- I take it that in the future, this method will take a Module*M<span class="apple-converted-space"> </span><br>parameter to specify which module to finalize. In order to not create<span class="apple-converted-space"> </span><br>future confusion in the C API, I expose this as<span class="apple-converted-space"> </span><br>LLVMFinalizeAllObjects() and specify the API's semantics as being that<span class="apple-converted-space"> </span><br>all objects associated with the execution engine should be finalized.<br><br>The patch also exposes the NoFramePointerElim option. The manner in<span class="apple-converted-space"> </span><br>which options are exposed is designed for forward compatibility; you<span class="apple-converted-space"> </span><br>supply an options struct along with a size which you zero-fill prior<span class="apple-converted-space"> </span><br>to<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">manipulating.<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">This is similar to the idiom I've seen used in other C APIs like<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">BerkeleyDB.<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">I considered having separate C function calls for each option, in the<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">style<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">of the ExecutionEngineBuilder API - but while that idiom feels right<span class="apple-converted-space"> </span><br>to me in C++, it feels less C-like. As well, the current options<span class="apple-converted-space"> </span><br>approach<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">exposes<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">not just parts of the Builder but also part of TargetOptions (namely,<span class="apple-converted-space"> </span><br>NoFramePointerElim). It's also more concise in practice.<br><br>I plan to expose more innards through the LLVMMCJITCompilerOptions in<span class="apple-converted-space"> </span><br>the future. I'd be happy to do more of that in one go if that was<span class="apple-converted-space"> </span><br>preferred; but I thought that a baby step would be the best thing for now.<br><br><br><br>-Filip<br><br><br>On Apr 21, 2013, at 6:26 PM, Filip Pizlo <<a href="mailto:fpizlo@apple.com" style="color: purple; text-decoration: underline;">fpizlo@apple.com</a>> wrote:<br><br><br>On Apr 15, 2013, at 10:42 AM, "Kaylor, Andrew"<span class="apple-converted-space"> </span><br><<a href="mailto:andrew.kaylor@intel.com" style="color: purple; text-decoration: underline;">andrew.kaylor@intel.com</a>><br>wrote:<br><br>OK, let me start by saying that MCJIT does take ownership of the<span class="apple-converted-space"> </span><br>memory manager. It doesn't use an OwningPtr, which would make this<span class="apple-converted-space"> </span><br>clear, but it does delete the pointer in its destructor. I think this<span class="apple-converted-space"> </span><br>is happening because we needed some finer control over when the MM got<span class="apple-converted-space"> </span><br>deleted. I<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">should<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">probably revisit this and at least add some comments to make it clear<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">what's<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">happening and why. It might not even be an issue anymore, because I<span class="apple-converted-space"> </span><br>did some work a while ago to try to clean up object ownership issues.<br><br><br>Actually, I was just confused. MCJIT deletes MemMgr, which is always<span class="apple-converted-space"> </span><br>aliased to Dyld, as far as I can tell. So I was just wrong. :-)<br><br><br>That said, I have been meaning for some time to break apart the JIT<span class="apple-converted-space"> </span><br>and MCJIT interfaces. The fact that they are both abstracted by<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">ExecutionEngine<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">and EngineBuilder complicates that, but it really needs to be done (as<span class="apple-converted-space"> </span><br>you are seeing).<br><br>For now, would it be possible to have the C-interface provide a<span class="apple-converted-space"> </span><br>wrapper<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">that<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">supplies empty implementations of the irrelevant functions when<span class="apple-converted-space"> </span><br>creating a memory manager for MCJIT?<br><br><br>I think this is sensible. I will proceed in this way.<br><br>Thanks!<br><br><br>-Andy<br><br>From: Filip Pizlo [<a href="mailto:fpizlo@apple.com" style="color: purple; text-decoration: underline;">mailto:fpizlo@apple.com</a>]<br>Sent: Saturday, April 13, 2013 1:18 AM<br>To: Kaylor, Andrew<br>Cc:<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;">llvm-commits@cs.uiuc.edu</a><br>Subject: Re: [PATCH][llvm-c] Expose MC JIT<br><br>Ah - good thing you pointed this out. I just realized that my patch<span class="apple-converted-space"> </span><br>is wrong. Perhaps I can get some feedback on the best way to architect this.<br><br>Here's the problem:<br><br>- MCJIT does not take ownership of the memory manager. Hence<span class="apple-converted-space"> </span><br>allocating<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">one<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">in the constructor is wrong; it'll leak when MCJIT dies. But deleting<span class="apple-converted-space"> </span><br>the memory manager passed to MCJIT would be a change in behavior, and<span class="apple-converted-space"> </span><br>I'm not sure if it's in line with either what existing users expect or<span class="apple-converted-space"> </span><br>what was intended. Insofar as the JIT instance corresponds to<span class="apple-converted-space"> </span><br>ownership of<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">modules,<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">it feels like it shouldn't also take ownership of the memory manager;<span class="apple-converted-space"> </span><br>for example you might imagine wanting to throw away the MCJIT but keep<span class="apple-converted-space"> </span><br>the<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">code<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">it generated and continue to use the memory manager to track it - and<span class="apple-converted-space"> </span><br>eventually free it. But EngineBuilder currently claims that the<span class="apple-converted-space"> </span><br>ExecutionEngine takes ownership of the JMM - I'm assuming that this is<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">just<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">wrong documentation, and that EngineBuilder's use of the same JMM<span class="apple-converted-space"> </span><br>option<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">for<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">both JIT and MCJIT is just not right.<br><br>- I'd like to expose SectionMemoryManager and, eventually in a<span class="apple-converted-space"> </span><br>separate patch, the ability to create custom RTDyldMemoryManagers via the C API.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">I'd<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">prefer this to be an RTDyldMemoryManager and not a JITMemoryManager,<span class="apple-converted-space"> </span><br>since the latter has a load of methods that are not relevant to MCJIT. <br>But EngineBuilder wants a JITMemoryManager. This would mean that the<span class="apple-converted-space"> </span><br>C API would have to pass its RTDyldMemoryManager via a cast to<span class="apple-converted-space"> </span><br>JITMemoryManager just so MCJIT could then use it as an<span class="apple-converted-space"> </span><br>RTDyldMemoryManager again. Seems wrong. I'm assuming that the<span class="apple-converted-space"> </span><br>correct long-term thing is to fix the EngineBuilder to not pass the<span class="apple-converted-space"> </span><br>JMM to the MCJIT, since it's good to expose the fact that the MCJIT<span class="apple-converted-space"> </span><br>actually just wants an RTDyldMemoryManager<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">instead.<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><br>In short, I'd like to have a separate EngineBuilder setting for the<span class="apple-converted-space"> </span><br>RTDyldMemoryManager. If this is specified and you end up using the<span class="apple-converted-space"> </span><br>JIT<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">and<br><br><br><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">not MCJIT, you get an error. If you use the MCJIT, then the<span class="apple-converted-space"> </span><br>RTDyldMemoryManager option overrides the JMM option. Or something<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">similar.<br><br><br><o:p></o:p></div><p class="MsoNormal" style="margin: 0in 0in 12pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><br>Does that make sense?<br><br>-Filip<br><br><br>On Apr 12, 2013, at 5:38 PM, Filip Pizlo <<a href="mailto:fpizlo@apple.com" style="color: purple; text-decoration: underline;">fpizlo@apple.com</a>> wrote:<br><br><br>Thanks for the feedback! I will try this change and see what happens.<br><br>-Filip<br><br><br>On Apr 12, 2013, at 5:35 PM, "Kaylor, Andrew"<span class="apple-converted-space"> </span><br><<a href="mailto:andrew.kaylor@intel.com" style="color: purple; text-decoration: underline;">andrew.kaylor@intel.com</a>><br>wrote:<br><br>Hi Filip,<br><br>I'll take a closer look at your patches on Monday, but my initial<span class="apple-converted-space"> </span><br>input is that the default memory manager used should be<span class="apple-converted-space"> </span><br>SectionMemoryManager rather than the DefaultJITMemoryManager.<br><br>Thanks,<br>Andy<br><br>From:<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits-bounces@cs.uiuc.edu" style="color: purple; text-decoration: underline;">llvm-commits-bounces@cs.uiuc.edu</a><span class="apple-converted-space"> </span><br>[<a href="mailto:llvm-commits-bounces@cs.uiuc.edu" style="color: purple; text-decoration: underline;">mailto:llvm-commits-bounces@cs.uiuc.edu</a>] On Behalf Of Filip Pizlo<br>Sent: Friday, April 12, 2013 4:49 PM<br>To:<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;">llvm-commits@cs.uiuc.edu</a><br>Subject: Re: [PATCH][llvm-c] Expose MC JIT<br><br>Revised patches included.<br><br>I added additional ruggedizing to the LLVMCreateMCJITCompilerForModule<span class="apple-converted-space"> </span><br>function, so that if it detects that the passed struct is larger than<span class="apple-converted-space"> </span><br>expected, it reports an error instead of continuing.<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" style="color: purple; text-decoration: underline;">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" style="color: purple; text-decoration: underline;">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br><br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" style="color: purple; text-decoration: underline;">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><o:p></o:p></p><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" style="color: purple; text-decoration: underline;">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br><br><br><br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" style="color: purple; text-decoration: underline;">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></div></div></div></div></div></blockquote></div><br></div>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></blockquote></div><br></body></html>