<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;">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.<div><br></div><div>Here's the problem:</div><div><br></div><div>- 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.</div><div><br></div><div>- 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.</div><div><br></div><div>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.</div><div><br></div><div>Does that make sense?</div><div><br></div><div>-Filip</div><div><br></div><div><br><div><div>On Apr 12, 2013, at 5:38 PM, Filip Pizlo <<a href="mailto:fpizlo@apple.com">fpizlo@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="auto" 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>Thanks for the feedback! I will try this change and see what happens. <br><br>-Filip</div><div><br>On Apr 12, 2013, at 5:35 PM, "Kaylor, Andrew" <<a href="mailto:andrew.kaylor@intel.com" style="color: purple; text-decoration: underline;">andrew.kaylor@intel.com</a>> wrote:<br><br></div><blockquote type="cite"><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);">Hi Filip,<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’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.<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);">Thanks,<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);">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><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>]<span class="Apple-converted-space"> </span><b>On Behalf Of<span class="Apple-converted-space"> </span></b>Filip Pizlo<br><b>Sent:</b><span class="Apple-converted-space"> </span>Friday, April 12, 2013 4:49 PM<br><b>To:</b><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<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 style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">Revised patches included.<o:p></o:p></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 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.<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></blockquote>_______________________________________________<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></blockquote></div><br></div></body></html>