[LLVMdev] Work in progress patch to bug 2606

Wed Feb 17 12:42:56 PST 2010

Hi Olivier,

Thanks for responding! I get to learn this way.

On Feb 17, 2010, at 12:50, Olivier Meurant wrote:

> Hi Garrison,
> 
> I am not a specialist of the code but here is my 2 cents:
> 
> - I like the idea that in lazy-mode the call (in module or not) is treated by a stub (like every calls).

If we go further with this, I'll have to add test cases for lazy mode. I kind of ignored it thinking I'll wait to see what
the group thinks about the approach.

> 
> - If the jit is in non-lazy mode, I'm not really fan of the "stub" solution. Is it not possible to use the same mechanism as it already exists : add the function to pending list and emit it after the current functions ? In fact, can you replace forEmitFunctionStub with getLazyFunctionStub ? With a non-lazy jit, I want the first execution of my function to be the same execution speed as the second one. A stub in non-lazy mode doesn't fell natural to me.

So as best as I can determine, the problem with leveraging pending, as used by getLazyFunctionStub(...) is that 
I don't know when to call JIT::updateFunctionStub(...). Ideally this behavior should be invoked in the outermost
getPointerToFunctionCall(), but one doesn't know, as currently implemented, when this is. 

To be clearer, in order to prevent recursive function compilation cases, one needs to delay the compilation of a 
function being called until after the current calling function is compiled. In other words to deal with the scenario 
where Fa (module A), calls Fb (module B), which in turn calls Fa, we delay the compilation of Fb until we have 
finished the compilation of  Fa, unless Fb has previously been compiled. Let's say we used JITEmitter::getLazyFunctionStub(...). 
If a user called getPointerToFunction(...) on Fa, everything would work fine. In compiling Fa, getPointerToFunction(...)
(via JIT::runJITOnFunctionUnlocked(...)) would encounter Fb in module B, run getLazyFunctionStub(...) on it, and 
then finish with the evaluation of the pending list; again via runJITOnFunctionUnlocked(). The latter functionality
would result in the compilation of Fb. Say however we instead had Fa1 call Fb, and Fa2 in sequence (Fa1, Fa2
in module A, Fa2 not previously compiled). The former description would still hold up to the point Fa2 is encountered. 
Since Fa2 is in Fa1's module, runJITOnFunctionUnlocked(...) would compile it, check the pending list, and update the 
contained functions. Since the stub for Fb is in that pending list, Fb would be compiled at this time. However the compilation 
of Fa1 is still not finished, and we have what I was trying to prevent. Any errors in my logic, or my understanding of the 
code? MutexGuards are just mutexes, right? Seems one could use reader/writer lock semantics to force the handing of 
the pending list only in the outmost runJITOnFunctionUnlocked(...) call. I should also add a test case for the above Fa1, 
Fa2, Fb, use case in the bug's attachments.

Having said this I might be going down a rat hole. It would seem such recursive compilations dependencies would exist
for functions in the same module, and these are handled. So I should probably look at that code. We are of course
limited by the existing code, since this is a bug fix not a design change. 

You also have another point. It is dissatisfactory to have the first call delayed just because it is in another module.
We could use another semantic where the pending list is handled outside of the outermost getPointerToFunction(...)
is called. This is of course a design change.

> 
> - I don't see why functions external to every modules (let's say "printf" ?) are delayed with your mechanism. In the last part of your modified getPointerToFunction, you call "getPointerToNamedFunction". AFAICT a printf call will be catch by this code, no ?

If I understand what your saying correctly: the printf will be caught here, but only after the module search is finished. In
essence I was saying that with the bug fix, as compared to the pre-existing implementation, the handling of such
external functions is delayed. If I could tell that the function belonged to this category before I did the module search
then there would obviously be no delay. Is this possible? I have to say I'm being rather theoretical here. Delay, no delay;
sounds like I'm trying to predict whether the tree fell in the forest. ;-) I just don't yet have the requisite LLVM knowledge to
run empirical tests. I tend to hide behind worries of falling timber given the erudite nature of this list's membership. :-)

> 
> - I like the ideas to be flexible, to be able to say this global variable declaration should point here (with addGlobalMapping), even if there is another module which have a global variable. And in fact, as the workaround is relatively easy, I wonder if it worth the change ?

I agree, I wonder if there should be a separate mode where a cross module find is turned on, with the default set to off. 
In the optional new mode, stubs would be gathered and processed (until done), as a separate phase--a linking phase
for JIT as exists in other LLVM JIT tools. I have not looked at those either.

1) Should we close the bug by dismissing it as no longer appropriate?
2) Apply the approach as given, with possible changes, and fix the bug?
3) Create a new approach, still viewing the bug as valid?
4) Keep the bug open because the implied features are desired but require a minor design change to resolve?
5) Do other? 

Thanks again for looking at this Olivier.

Garrison

> 
> Anyway, thanks for asking opinion on this and for contributions you've made on code !
> 
> Olivier.
> 
> 
> On Tue, Feb 16, 2010 at 2:10 PM, Garrison Venn <gvenn.cfe.dev at gmail.com> wrote:
> The patch I recently attached to bug 2606, reproduced here, is my first attempt to solve this issue
> as represented by the attached test cases. To solve the foreign Module GlobalVariable problem,
> I modified JIT::getOrEmitGlobalVariable(...) to directly attempt to map a found "external" GlobalVariable.
> To solve the foreign Module Function problem, I modified both JIT.{h,cpp} and JITEmitter.cpp to emit
> a stub for a found "external" Function instance, when that foreign Module defined Function has not
> yet been compiled.
> 
> All areas of interest are marked with // GMV Mod:
> 
> As I've taken liberties with existing code, and thereby may have touched test cases that will fail, I would like
> to have those members of the list that have time, review this proposal for comments/changes.
> 

[snip]

> Thanks for any time spent on this
> 
> Garrison
> 
> 
> 
> 	
> 
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100217/a413031f/attachment.html>