<div dir="rtl"><div dir="ltr">Ok, it's simpler than I thought since only three states are relevant.</div><div dir="ltr"><br></div><div dir="ltr">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?</div>

<div dir="ltr"><br></div><div dir="ltr">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").</div>

<div dir="ltr"><br></div><div dir="ltr">addModule emits module and adds the module to an "Compiled" vector of modules.</div><div dir="ltr">Finalize loops over "Compiled" vector, each module is finalized, added to "Linked" list and then the "Compiled" list is cleared.</div>

<div dir="ltr"><br></div><div dir="ltr">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.</div>

<div dir="ltr"><br></div><div dir="ltr">Yaron</div><div dir="ltr"><br></div><div dir="ltr"><br></div><div dir="ltr"><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote"><div dir="ltr">2013/10/18 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">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.<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">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.<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">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.<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">-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"><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()<u></u><u></u></span></p><div><div class="h5">
<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<br>
<br>
<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-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">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><br></div>