<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Hi Olivier,<div><br><div><div>On Feb 25, 2010, at 14:10, Olivier Meurant wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Hi Garrison,<br><br>I finally come back from holidays and take time to watch your patch.<br><br>I must say that I largely prefer this version over the previous one ! I like the reuse of getLazyFunctionStub, but I don't know if the forceEmitFunctionStub is still needed ?<br></blockquote><div><br></div>JIT::forceEmitFunctionStub(...) was created to bring its implementation into JITEmitter file scope as JITResolver and </div><div>therefore JITResolver::getLazyFunctionStub(...) are members of an anonymous namespace in that scope. However</div><div>I can instead use a pre-existing method: getPointerToFunctionOrStub(...) which is declared in ExecutionEngine</div><div>and overwritten in JIT (within JITEmitter file scope). Your response prompted my search. I've unit tested this and it works great. </div><div>There is one caveat though. The implementation of JIT::getPointerToFunctionOrStub(...) first invokes ExecutionEngine::getPointerToGlobalIfAvailable(...)</div><div>behavior to check whether or not the Function instance is already actualized. Subsequently JIT::getPointerToFunctionOrStub(...) </div><div>calls JITResolver::getLazyFunctionStub(...). Given that ExecutionEngine::getPointerToGlobalIfAvailable(...) iterates through all known </div><div>actualized functions, we would take a compile time hit here by using getPointerToFunctionOrStub(...) versus my forceEmitFunctionStub(...) </div><div>since we know at that point in the call stack that we are already dealing with a declaration. I don't know if this is a viable concern, </div><div>but if not I'd be happy to change the <span class="Apple-style-span" style="font-family: sans-serif; -webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 1px; "><a href="http://llvm.org/bugs/show_bug.cgi?id=2606" style="color: rgb(102, 51, 102); ">2606</a> working patch to reflect use of <span class="Apple-style-span" style="font-family: Helvetica; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; ">getPointerToFunctionOrStub(...) in place of forceEmitFunctionStub(...). </span></span></div><div><span class="Apple-style-span" style="font-family: sans-serif; -webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 1px; "><span class="Apple-style-span" style="font-family: Helvetica; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; ">On the pro side, using getPointerToFunctionOrStub(...) does get rid of introducing a new method (forceEmitFunctionStub(...)), which will not </span></span></div><div><span class="Apple-style-span" style="font-family: sans-serif; -webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 1px; "><span class="Apple-style-span" style="font-family: Helvetica; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; ">have the same future design support as getPointerToFunctionOrStub(...). We could also modify ExecutionEngine::getPointerToGlobalIfAvailable(...) to </span></span></div><div><span class="Apple-style-span" style="font-family: sans-serif; -webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 1px; "><span class="Apple-style-span" style="font-family: Helvetica; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; ">avoid the search if the function is a declaration. I personally don't care for this approach.</span></span></div><div><br></div><div><blockquote type="cite">
<br>I thought about JIT and modules, and I wonder if we don't need to take it another way.<br>Now we can create multiples JIT. What if we decide to simplify the JIT in allowing only one module per instance of JIT ? It will simplify greatly the code of the JIT.<br>
<br>If we need to jit 2 differents modules, we can create 2 JITs. If one module needs another module, we can create a LazyFunctionCreator (called by JIT::getPointerToNamedFunction) and easily resolved cross-module references.<br></blockquote><div><br></div>I can't comment on this as I have not dealt with multiple JITs as you and Jeffrey have (for those reading this thread after the fact </div><div>see Jeffrey's follow up response).</div><div><br></div><div>Like you though I am favoring a solution where the factory nature of EngineBuilder is leveraged. In my view we should be able</div><div>to create a desired ExecutionEngine via this pattern. In reality though, there are two existing factory semantics used for the </div><div>construction of ExecutionEngines. One is the one given by the API in EngineBuilder, where the type of ExecutionEngine is</div><div>determined by the a priori EngineBuilder::Kind. The other is contained by the manual header file include mechanism; including </div><div>llvm/include/ExecutionEngine/JIT.h and or llvm/include/ExecutionEngine/ExecutionEngine.h. Currently both of these methods</div><div>are used in conjunction to determine the final outcome. If however one wanted to include a derived class of JIT to optionally bring </div><div>into being my behavioral fix for <span class="Apple-style-span" style="font-family: sans-serif; -webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 1px; "><a href="http://llvm.org/bugs/show_bug.cgi?id=2606" style="color: rgb(102, 51, 102); ">2606</a>, one would have to deal with these methods. I personally would create<span class="Apple-style-span" style="font-family: Helvetica; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; "> a new inlined virtual </span></span></div><div><span class="Apple-style-span" style="font-family: sans-serif; -webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 1px; "><span class="Apple-style-span" style="font-family: Helvetica; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; ">declaration in JIT, and use it in JIT::getPointerToFunction(...), in place of my <span class="Apple-style-span" style="font-family: sans-serif; -webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 1px; "><a href="http://llvm.org/bugs/show_bug.cgi?id=2606" style="color: rgb(102, 51, 102); ">2606</a> mod, to effect the new cross module functionality</span></span></span></div><div><span class="Apple-style-span" style="font-family: sans-serif; -webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 1px; "><span class="Apple-style-span" style="font-family: Helvetica; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; "><span class="Apple-style-span" style="font-family: sans-serif; -webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 1px; ">in this new JIT derived class instance. A simple hack resolution of "resolve these methods" implies leveraging another include file </span></span></span></div><div><span class="Apple-style-span" style="font-family: sans-serif; -webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 1px; "><span class="Apple-style-span" style="font-family: Helvetica; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; "><span class="Apple-style-span" style="font-family: sans-serif; -webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 1px; ">to set ExecutionEngine::JITCtor while simultaneously handling JIT.cpp's static setting of this same variable. A better solution would </span></span></span></div><div><span class="Apple-style-span" style="font-family: sans-serif; -webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 1px; "><span class="Apple-style-span" style="font-family: Helvetica; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; "><span class="Apple-style-span" style="font-family: sans-serif; -webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 1px; ">be to redesign EngineBuilder to use say dlopen/dlload functionality to dynamically pull in the proper JIT derived class library. </span></span></span></div><div><span class="Apple-style-span" style="font-family: sans-serif; -webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 1px; "><span class="Apple-style-span" style="font-family: Helvetica; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; "><span class="Apple-style-span" style="font-family: sans-serif; -webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 1px; ">Anyway this is my 2 cents.</span></span></span></div><div><br></div><div><blockquote type="cite">
<br>What do you think ? Anyone have an opinion on this ?<br><br>Olivier.<br></blockquote><div><br></div>Garrison</div><div><br><blockquote type="cite"><br><div class="gmail_quote">On Fri, Feb 19, 2010 at 5:21 PM, Garrison Venn <span dir="ltr"><<a href="mailto:gvenn.cfe.dev@gmail.com">gvenn.cfe.dev@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div style="word-wrap: break-word;">This is the second version of a patch, which I recently attached to bug <span style="font-family: sans-serif;"><a href="http://llvm.org/bugs/show_bug.cgi?id=2606" style="color: rgb(102, 51, 102);" target="_blank">2606</a>, whose original version was</span><div>
<font face="sans-serif"><span>modified to reflect the lists comments. Also please note the comment at the end of this email, which basically</span></font></div><div><font face="sans-serif"><span>questions whether this bug is really a bug.<br>
</span></font><div><font face="sans-serif"><span><br></span></font></div><div><font face="sans-serif"><span>1) To solve the foreign Module GlobalVariable problem, I modified JIT::getOrEmitGlobalVariable(...) to </span></font></div>
<div><font face="sans-serif"><span><span style="white-space: pre;"> </span>directly attempt to map a found "external" GlobalVariable. </span></font></div><div><font face="sans-serif"><span>2) To solve the foreign Module Function problem, </span></font></div>
<div><font face="sans-serif"><span><span style="white-space: pre;"> </span>I modified JIT.{h,cpp} to emit a stub for a found "external" Function instance, when that foreign Module </span></font></div><div><font face="sans-serif"><span><span style="white-space: pre;"> </span>defined Function has not yet been compiled.</span></font></div>
<div><font face="sans-serif"><span><br></span></font></div><div><font face="sans-serif"><span>All areas of interest are marked with // GMV Mod:</span></font></div><div><font face="sans-serif"><span><br></span></font></div>
<div><font face="sans-serif"><span>Again I invite those members of the list that have time, to review this proposal for comments/changes.</span></font></div><div><font face="sans-serif"><span><br></span></font></div><div>
<font face="sans-serif"><span>Major mods:</span></font></div><div><font face="sans-serif"><span><br></span></font></div><div><font face="sans-serif"><span>JIT.{h,cpp}:</span></font></div><div><font face="sans-serif"><span><br>
</span></font></div><div><font face="sans-serif"><span>1)<span style="white-space: pre;"> </span>I included Module.h.</span></font></div><div><font face="sans-serif"><span><br></span></font></div><div><font face="sans-serif"><span>2)<span style="white-space: pre;"> </span>JIT::<span style="white-space: pre;">getPointerToFunction</span>(...) was modified to search for a non-mapped function in all "other" modules.</span></font></div>
<div><font face="sans-serif"><span><span style="white-space: pre;"> </span>If found a stub is emitted in the same manner as other in module functions found. This stub gets re-emitted, </span></font></div><div><font face="sans-serif"><span><span style="white-space: pre;"> </span>toward the end of the JIT cycle.</span></font></div>
<div><font face="sans-serif"><span><br></span></font></div><div><font face="sans-serif"><span><span style="white-space: pre;"> </span>One issue here is that functions that are external to all modules, have their mappings delayed by the</span></font></div>
<div><font face="sans-serif"><span><span style="white-space: pre;"> </span>above loop. I also kept the hasAvailableExternallyLinkage() the same as before, as I'm not sure if this</span></font></div><div><font face="sans-serif"><span><span style="white-space: pre;"> </span>should also go through the same module procedure. The stub emission will only take place for function</span></font></div>
<div><font face="sans-serif"><span><span style="white-space: pre;"> </span>definitions that are marked with external linkage. Should other cases be allowed? Should visibility also </span></font></div><div><font face="sans-serif"><span><span style="white-space: pre;"> </span>be considered?</span></font></div>
<div><font face="sans-serif"><span><br></span></font></div><div><font face="sans-serif"><span>3)<span style="white-space: pre;"> JIT::getOrEmitGlobalVariable</span>(...) was modified to search for non-mapped global variables in all "other"</span></font></div>
<div><font face="sans-serif"><span><span style="white-space: pre;"> </span>modules. If found, the address of the foreign global variable is requested, and mapped. There is no</span></font></div><div><font face="sans-serif"><span><span style="white-space: pre;"> </span>delay here.</span></font></div>
<div><font face="sans-serif"><span><br></span></font></div><div><font face="sans-serif"><span><span style="white-space: pre;"> </span>An attempt is first made to find the global outside of all modules, before attempting to find the function</span></font></div>
<div><font face="sans-serif"><span><span style="white-space: pre;"> </span>in the known modules. This is the reverse of logic used in #2. On searching, unlike for functions, a linkage </span></font></div><div><font face="sans-serif"><span><span style="white-space: pre;"> </span>check was not made. Also the global variable declaration and definition must have the exact same type. </span></font></div>
<div><font face="sans-serif"><span><span style="white-space: pre;"> </span>Is this the correct approach?</span></font></div><div><font face="sans-serif"><span><br></span></font></div><div><font face="sans-serif"><span>4) The declaration of void* forceEmitFunctionStub(Function *F) was added to the class JIT. This is </span></font></div>
<div><font face="sans-serif"><span><span style="white-space: pre;"> </span>implemented in JITEmitter.cpp.</span></font></div><div><font face="sans-serif"><span><br></span></font></div><div><span style="font-family: sans-serif;">JITEmitter.cpp:</span></div>
<div><font face="sans-serif"><span><br></span></font></div><div><font face="sans-serif"><span>1)<span style="white-space: pre;"> JIT::forceEmitFunctionStub</span>(...) was added. It turns around and calls <span style="font-family: Helvetica;"><span style="font-family: sans-serif;">JITResolver::getLazyFunctionStub(...) which</span></span></span></font></div>
<div><font face="sans-serif"><span><span style="white-space: pre;"> </span>emits the foreign function as a stub.</span></font></div><div><font face="sans-serif"><span><br></span></font></div><div><font face="sans-serif"><span>Beyond any issues with the patch, there is a question, in my mind, as to whether <a href="http://llvm.org/bugs/show_bug.cgi?id=2606" style="color: rgb(102, 51, 102);" target="_blank">2606</a> is really a bug. Sure its resolution makes </span></font></div>
<div><font face="sans-serif"><span>using the JIT simpler for cross module behavior, but current manual solutions may be more fined grained in their approach in</span></font></div><div><font face="sans-serif"><span>deciding how to treat such functions. If true a fix to <a href="http://llvm.org/bugs/show_bug.cgi?id=2606" style="color: rgb(102, 51, 102);" target="_blank">2606</a> would circumvent such handling. Should we instead add a new </span></font></div>
<div><font face="sans-serif"><span>semantic to EngineBuilder which would configure the JIT, or whatever else, to handle such cross module linkage, if such behavior</span></font></div><div><font face="sans-serif"><span>is warranted? By linkage I mean mapping variables, and functions found in "external" modules, into the execution engine. For example, </span></font></div>
<div><font face="sans-serif"><span>one could devise a new function pass style metaphor which would be setup in the EngineBuilder instance. If it exists, the resulting</span></font></div><div><font face="sans-serif"><span>JIT instance would use this new pass style to determine what to do with "foreign functions, and variables". However the first question </span></font></div>
<div><font face="sans-serif"><span>is, whether or not <a href="http://llvm.org/bugs/show_bug.cgi?id=2606" style="color: rgb(102, 51, 102);" target="_blank">2606</a> is a bug in the first place?</span></font></div><div><font face="sans-serif"><span><br>
</span></font></div><div><font face="sans-serif"><span>Thanks again for any time spent on this</span></font></div><div><font face="sans-serif"><span><br></span></font></div><font color="#888888"><div><font face="sans-serif"><span>Garrison</span></font></div>
<div><font face="sans-serif"><span><br></span></font></div><div><font face="sans-serif"><span></span></font></div></font></div></div><br><div style="word-wrap: break-word;"><div><br><div><font face="sans-serif"><span></span></font></div>
<div><font face="sans-serif"><span><br></span></font></div><div><font face="sans-serif"><span><span style="white-space: pre;"> </span></span></font></div><div><font face="sans-serif"><span><br></span></font></div></div></div>
<br>_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu/" target="_blank">http://llvm.cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
<br></blockquote></div><br>
</blockquote></div><br></div></body></html>