[LLVMdev] Work in progress patch to bug 2606

[Olivier Meurant]

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 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.

- 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 ?

- 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 ?

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<http://llvm.org/bugs/show_bug.cgi?id=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.
>
> Major mods:
>
> JIT.{h,cpp}:
>
> 1) I included Module.h.
>
> 2) JIT::getPointerToFunction(...) was modified to search for a non-mapped
> function in all "other" modules.
> If found a stub is emitted, which will lazily emit the real function at
> runtime.
>
> One issue here is that functions that are external to all modules, have
> their mappings delayed by the
> above loop. I also kept the hasAvailableExternallyLinkage() the same as
> before, as I'm not sure if this
> should also go through the same module procedure. The stub emission will
> only take place for function
> definitions that are marked with external linkage. Should other cases be
> allowed? Should visibility also
> be considered?
>
> 3) JIT::getOrEmitGlobalVariable(...) was modified to search for non-mapped
> global variables in all "other"
> modules. If found, the address of the foreign global variable is requested,
> and mapped. There is no
> delay here.
>
> An attempt is first made to find the global outside of all modules, before
> attempting to find the function
> in the known modules. This is the reverse logic used in #2. On searching,
> unlike for functions, a linkage
> check was not made. Also the global variable declaration and definition
> must have the exact same type.
> Is this the correct approach?
>
> 4)    The declaration of void* forceEmitFunctionStub(Function *F) was added
> to the class JIT. This is
> implemented in JITEmitter.cpp.
>
> JITEmitter.cpp:
>
> 1) JIT::forceEmitFunctionStub(...) was added. It turns around and calls
> JITResolver::forceEmitFunctionStub(...).
>
> Is the JIT:getPointerToFunction(...) code path sensitive to such call
> overhead?
>
> 2) JITResolver::forceEmitFunctionStub(Function *F) was added. It is a
> simplified version of JITResolver::getLazyFunctionStub(...),
> which does not add to pending. It uses the
> same JITResolver::JITCompilerFn(...) for runtime emission that
> the lazy compilation system uses.
>
> Outside of the goal, I'm not sure what code is unnecessary here. Notice
> that the hasAvailableExternallyLinkage
> case is dealt with here even though forceEmitFunctionStub(...) would not
> be entered for this case. I'm forced to
> use JITResolver::JITCompilerFn(...) as there is a static relationship
> between JITResolver and say X86JITInfo. See
> implementation of X86JITInfo::getLazyResolverFunction(...)
> (X86JITInfo.cpp). Should a new JITCompilerFn like
> function be created for the purposes of resolving this issue? This would of
> course require modifications to the
> subclasses of TargetJITInfo.
>
> 3) JITResolver::JITCompilerFn(...) was modified to no longer exit on being
> used for non-lazy compilation scenarios.
>
> This means of course that the error test path behavior for this function
> has been modified. See #2 above.
>
> Beyond any issues with the patch, there is a question, in my mind, as to
> whether 2606 <http://llvm.org/bugs/show_bug.cgi?id=2606> is really a bug.
> Sure its resolution makes
> using the JIT simpler for cross module behavior, but current manual
> solutions may be more fined grained in their approach.
> If so a fix to 2606 <http://llvm.org/bugs/show_bug.cgi?id=2606> would
> circumvent such handling.
>
> 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/83d76f0b/attachment.html>