[LLVMdev] 2nd attempt for a working patch for bug 2606

[Garrison Venn]

This is the second version of a patch, which I recently attached to bug 2606, whose original version was
modified to reflect the lists comments. Also please note the comment at the end of this email, which basically
questions whether this bug is really a bug.

1) To solve the foreign Module GlobalVariable problem, I modified JIT::getOrEmitGlobalVariable(...) to 
	directly attempt to map a found "external" GlobalVariable. 
2) To solve the foreign Module Function problem, 
	I modified JIT.{h,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:

Again I invite those members of the list that have time, to 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 in the same manner as other in module functions found. This stub gets re-emitted, 
	toward the end of the JIT cycle.

	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 of 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::getLazyFunctionStub(...) which
	emits the foreign function as a stub.

Beyond any issues with the patch, there is a question, in my mind, as to whether 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 in
deciding how to treat such functions. If true a fix to 2606 would circumvent such handling. Should we instead add a new 
semantic to EngineBuilder which would configure the JIT, or whatever else, to handle such cross module linkage, if such behavior
is warranted? By linkage I mean mapping variables, and functions found in "external" modules, into the execution engine. For example, 
one could devise a new function pass style metaphor which would be setup in the EngineBuilder instance. If it exists, the resulting
JIT instance would use this new pass style to determine what to do with "foreign functions, and variables". However the first question 
is, whether or not 2606 is a bug in the first place?

Thanks again for any time spent on this

Garrison



	

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100219/316ad368/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: PR2606.proposal.patch
Type: application/octet-stream
Size: 6962 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100219/316ad368/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100219/316ad368/attachment-0001.html>