<div dir="rtl"><div dir="ltr">Hi,</div><div dir="ltr"><br></div><div dir="ltr">Sure, I'll do the SmallPtrSet patch. </div><div dir="ltr"><br></div><div dir="ltr">I'll get to the other issues later.</div><div dir="ltr">
<br></div><div dir="ltr">Yaron</div><div dir="ltr"><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote"><div dir="ltr">2013/10/21 Kaylor, Andrew <span dir="ltr"><<a href="mailto:andrew.kaylor@intel.com" target="_blank">andrew.kaylor@intel.com</a>></span></div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div lang="EN-US" link="blue" vlink="purple">
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Yaron,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Regarding the name changes, my expectation is that typically the module(s) will have already been loaded before the finalize method is called. I was thinking
of a name more like “prepareModuleForExecution”.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Regarding the data structure, I had considered the linear time find issue and I did look over the data structures in the llvm programmer’s manual before choosing
the list. Mostly I chose it because it seemed like the simplest way to implement what I wanted. I wasn’t worried about the find time because the only case where find is used outside of assert statements is in the deprecated getPointerToFunction method.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">That said, I’m not against the suggestion of using SmallPtrSet. I was hoping to provide an interface to the module container class that would allow the core
MCJIT implementation to be independent of the container class implementation so it could be optimized as needed without requiring changes to the base MCJIT code. As I presented it in the patch using three internal containers would require a custom iterator
implementation to walk the full set of containers, but that could be avoided by making the changes you suggest.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Would you be interested in taking over this patch and working it through to completion? I’ve got some other things I need to work on, and it would be very
helpful to me if you could do this.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Thanks,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Andy<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">P.S. One other thing that I noticed while considering your comments…. The old getPointerToFunction method takes an llvm::Function pointer as input, while the
new getFunctionAddress takes the name of the function. There are a number of reasons for this change. We want MCJIT to be capable of discarding IR objects once code has been generated. Also, there will be cases where MCJIT has never had IR for loaded objects.
(I have a patch in the works for this latter case now.) For these reasons, we need an interface that will find symbol addresses with just a symbol name. However, I noticed while considering your comments that searching for an owning module using a function
name is necessarily a linear search, whereas if we had the llvm::Function object from which we can get the parent module directly, it would just be a simple matter of verifying that we own that module.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Perhaps both forms of the function are worth having. On the other hand, the search by name is always going to happen when we’re linking so we should probably
try to find a way to optimize that path. One possibility would be to build some kind of map of names to modules as modules are added. This would add a lot of upfront cost and wouldn’t be worth doing in many cases, but in the case of a REPL that is likely
to have a low function-to-module ratio, contain a lot of modules and require a lot of linking this might be a critical optimization. My suggestion here would be to provide a client option to enable this sort of behavior.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u><u></u></span></p>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Yaron Keren [mailto:<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>]
<br>
<b>Sent:</b> Saturday, October 19, 2013 1:58 AM</span></p><div><div class="h5"><br>
<b>To:</b> Kaylor, Andrew<br>
<b>Cc:</b> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> Re: ExecutionEngine::finalizeModule()<u></u><u></u></div></div><p></p><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">Hi,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Couple of ideas how to fix the FIXMEs.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">finalizeModule should be called loadAndFinalizeModule <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">and<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">finalizeObject should be called loadAndFinalizeModules<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">to convey their functions.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">About the data structures, while the combined list solves nicely the looping find case it introduces a linear time find is time linear with possibly a high constant: <a href="http://llvm.org/docs/ProgrammersManual.html#list" target="_blank">http://llvm.org/docs/ProgrammersManual.html#list</a> says "std::list
is an extremely inefficient class that is rarely useful. It performs a heap allocation for every element inserted into it, thus having an extremely high constant factor, particularly for small data types."<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">LLVM has many excellent datatypes available.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">The SmallPtrSet <a href="http://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallptrset-h" target="_blank">http://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallptrset-h</a> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">would be a great fit for MCJIT needs: Iteration is still supported and other operations are constant time.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">This will make a big difference when there are 1000s of modules.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">The price we'd have to pay for using SmallPtrSet is that we'll have three different SmallPtrSets so any operation on all three sets will require three loops, set by set.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">How bad is it ? there are two cases where a loop on all owned modules is required:<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">1) In MCJIT.CPP, we need several times to find a module from the three sets.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">This can be done by a find function of the OwningModuleContainer which will find in the three sets and using it.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">2) in ~OwningModuleContainer<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">We will need to destruct the three sets in three loops.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Not too bad in exchange for runtime performance that will scale much better to the number of modules.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Yaron<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">2013/10/19 Kaylor, Andrew <<a href="mailto:andrew.kaylor@intel.com" target="_blank">andrew.kaylor@intel.com</a>><u></u><u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Yaron,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Here’s a first attempt at implementing what I described below. It’s still got one significant place
that I know needs revision (marked by a FIXME comment), but it seems to be functional.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Would you mind checking it out and telling me what you think?</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Thanks,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Andy</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Yaron Keren [mailto:<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>]
<br>
<b>Sent:</b> Friday, October 18, 2013 3:04 PM</span><u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><br>
<b>To:</b> Kaylor, Andrew<br>
<b>Cc:</b> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> Re: ExecutionEngine::finalizeModule()<u></u><u></u></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">This data structure sounds good .<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">I actually didn't consider lazy compilation with regard about module states because I knew that "MCJIT does not have lazy compilation". So the loaded state purpose is having modules
in "standby" should they be required. OK. <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">If you put one function into one module exactly (call this a function-module) you almost have the lazy compilation capability. Maybe it's possible to split a module into function-modules
automatically in a addModuleFunctions() function.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Some questions -<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">1) Is the purpose of having loaded and finalized states to enable an external action on the module between loading and finalizing?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">2) Is it OK to finalize a set of modules, add more modules and finalize again?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Yaron<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">2013/10/19 Kaylor, Andrew <<a href="mailto:andrew.kaylor@intel.com" target="_blank">andrew.kaylor@intel.com</a>><u></u><u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I’ve actually got an idea for this and am working on a patch today. I’ll send it to you for input
when I have something working.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Basically my idea is to replace the existing Modules vector that MCJIT inherits from ExecutionEngine
with a smarter container class that will internally maintain separate lists/sets for added, loaded and finalized modules. This class would provide iterator access that MCJIT could use to iterate through the entire combined set of modules (I think I can make
that work reasonably) but also iterators to go through just the modules in a particular state. This would let us get rid of the clumsy ModuleState thing completely and provide faster access to the subsets we need.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">The “loaded” state is needed because when relocations are processed (during the finalize handling)
they are applied to all modules that have been loaded and the finalize operation on the memory manager (which sets page protection, invalidates code caches, etc.) is applied to the sections of all loaded modules. It would require a significant amount of extra
infrastructure to make this behave otherwise and I couldn’t think of a compelling reason why it would be needed.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">But maybe there’s some confusion here. The emitted state is entirely useless because it is always
immediately followed by object loading. The state I’m currently calling “loaded” is probably what you’re referring to as “compiled”. We definitely do not want to immediately compile modules when addModule is called because that would eliminate the only facsimile
of lazy compilation MCJIT currently has. The idea is that you can add a module to provide function definitions but if nothing in that module is referenced by a module in which a function is executed (note the wording here because it’s not exactly the ideal
case) then the module never gets compiled.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Eventually, we’ll want to add another level of lazy compilation in which we generate stubs to resolve
external function during linking/finalization and only compile their dependent modules if the stub is called. But that’s a job for another day.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">-Andy</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Yaron Keren [mailto:<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>]
<br>
<b>Sent:</b> Friday, October 18, 2013 1:41 PM</span><u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><br>
<b>To:</b> Kaylor, Andrew<br>
<b>Cc:</b> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> Re: ExecutionEngine::finalizeModule()<u></u><u></u></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">Ok, it's simpler than I thought since only three states are relevant.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">But first - do we need really all three states? what good is loaded state? is just means being on the list, and any operation that is possible on a loaded module should be possible
before adding the module to MCJIT, right?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">If we merge loading and emitted by having the addModule immediately emit the loaded module we'll need only two states: "Compiled" and "Linked" (or "Finalized").<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">addModule emits module and adds the module to an "Compiled" vector of modules.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Finalize loops over "Compiled" vector, each module is finalized, added to "Linked" list and then the "Compiled" list is cleared.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">If we still need three lists then just the same, three vectors would be required with add inserting the loaded vector, emit moving modules from loaded to compiled and finalize moving
them from compiled to linked.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Yaron<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">2013/10/18 Kaylor, Andrew <<a href="mailto:andrew.kaylor@intel.com" target="_blank">andrew.kaylor@intel.com</a>><u></u><u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Your suggestion to introduce an optimization within MCJIT to avoid inefficiency in finalizeLoadedModules
is the correct direction. I definitely prefer to improve the MCJIT implementation rather than increasing the surface of the interface and requiring clients to optimize their own use of MCJIT.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">We’ve talked about removing runFunction from the MCJIT interface. It really doesn’t belong there
and was just implemented as a convenience to ease the transition of legacy code to MCJIT. As such, I think of it in the same way I think of finalizeObject. That is, I’m hoping it will go away. It might make sense to provide an independent class or perhaps
a static function that provides the signature decoding that’s contained in MCJIT::runFunction, which would ease the pain of deprecating it. That method isn’t entirely complete as it stands, BTW.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">The Module state handling in MCJIT definitely isn’t mature. The only states that really have any
significance are “Added” (which needs a better name), “Loaded” and “Finalized”. The current state tracking is just something I put in there to get the multiple module support up and running. I would be happy to see it replaced with something better.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">-Andy</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Yaron Keren [mailto:<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>]
<br>
<b>Sent:</b> Thursday, October 17, 2013 4:02 PM<br>
<b>To:</b> Kaylor, Andrew<br>
<b>Cc:</b> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> Re: ExecutionEngine::finalizeModule()</span><u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">The changes you are making make sense. Return valid ready-to-run function address rather than depend upon the user calling finalize is right.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">The issue is with scalability of the current implementation. Let's take your Kaliedoscope example (which was very helpful) as the use-case but every REPL will do the same.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">After adding a new module it and only it needs to be finalized to run the new code. But finalizeLoadedModules loops over all modules to see which one should be finalized. It's
not efficient. <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">I initially thought using finalizeModule would be the solution, calling it after creating a module, saving the loop.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">However this goes deeper and finalizeModule isn't the solution. As you say getFunctionAddress calls finalizeLoadedModules() if a symbol found. finalizeLoadedModules always loops
on all modules so it's wasteful. In an extreme case, we may know that all modules are already finalized but can't prevent the needless loop on module.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">The solution would be other data structures. <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Simplified case, instead of looping on all modules and checking module states MCJIT could have a vector of un-finalized modules and a vector of finalized modules. New modules enter
the un-finalized vector and finalizeLoadedModules() loops over the un-finalized vector, finalize every module, appends them to finalized vector and clears the unfinalized vector. <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt">This is simplified since there are more ModuleStates so maybe other data structures should be used. I have not looked in depth the ModuleStates.<br>
<br>
Regarding getFunctionAddress, runFunction() does not use it but calls the older getPointerToFunction, so either runFunction() should be changed to the newer API or we still need to manually finalize modules.<br>
<br>
Yaron<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">2013/10/18 Kaylor, Andrew <<a href="mailto:andrew.kaylor@intel.com" target="_blank">andrew.kaylor@intel.com</a>><u></u><u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Yaron,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">What do you see as the use case for adding this? I was thinking about removing finalizeObject()
from the interface -- it’s mostly there to support code that was written when it was necessary. It was probably a misstep to introduce finalizeObject in the EE interface in the first place. It’s a case of leaking an implementation detail into the interface.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">With the current trunk implementation of MCJIT, calling getFunctionAddress will implicitly finalize
the module that contains the function (as well as any other modules that have been loaded). The idea is that the address you get back is immediately useful (which was not true of the older getPointerToFunction method).</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Also note that within MCJIT calling finalizeModule has the effect of finalizing all loaded modules.
In fact, I plan to update the implementation of finalizeModule to call finalizeLoadedModules soon.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">-Andy</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Yaron Keren [mailto:<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>]
<br>
<b>Sent:</b> Thursday, October 17, 2013 2:32 PM<br>
<b>To:</b> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>; Kaylor, Andrew<br>
<b>Subject:</b> ExecutionEngine::finalizeModule()</span><u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">Hi,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Make finalizeModule() accessible for MCJIT created through ExecutionEngine.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Maybe it would make sense to expose finalizeLoadedModules() as well.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Yaron<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div></div></div>
</div>
</blockquote></div><br></div>