[LLVMdev] Work in progress patch to bug 2606

[Jeffrey Yasskin]

On Wed, Feb 17, 2010 at 12:42 PM, Garrison Venn <gvenn.cfe.dev at gmail.com> wrote:
> 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.

Sorry for being so slow at looking at this. The existing pending
function system ensures that only one function is being compiled at a
time, and enforces that with the "assert(!isAlreadyCodeGenerating)" at
the top of runJITOnFunctionUnlocked(). You should use that mechanism
instead of adding a new way to create stubs.

> 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
>>
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>