[llvm-commits] [PATCH] unit tests for MCJIT

[Malea, Daniel]

Hi Jim,

Thanks again for the review comments; I addressed them with the following changes:
- removed SectionMemoryManager from public headers and added it to unittests/ExecutionEngine/MCJIT instead
- added a one-sentence description of each file added below the license header
- fixed style issues with placement of '*' and spacing around '='

Please find the updated patches attached. They no longer touch anything outside unittests.



Thanks,
Dan

On 2012-09-24, at 1:37 PM, Jim Grosbach wrote:

> Hi Daniel,
>
> Sorry for the delay in getting to this. Thank you for doing this. I'm really glad to see new tests being added for this stuff. A few minor comments:
>
> The comment block at the top of each file should contain a brief (one or two sentence) description of the purpose of the file immediately below the license boilerplate. SectionMemoryManager.h has this, but the others are missing it.
>
> The most substantive comment is that we shouldn't move the memory manager out of lli.cpp. That's very much not a general purpose memory manager, but rather is intended as a (trivial) example of what a client application may need to implement in order to utilize the MCJIT. It's not part of a general LLVM library, but rather part of the lli tool.
>
> In a few places, the '*' on declarations is incorrect according to LLVM style conventions. For example,
> void* addPtr = TheJIT->getPointerToFunction(F);
>
> The '*' should bind to the identifier, not the type. e.g., "void *addPtr".
>
> Similarly,
> +  int (*FuncPtr)(void) =(int(*)(void))(intptr_t)vPtr;
>
> There should be a space after the '='.
>
> Regards,
>  Jim
>
>
> On Aug 29, 2012, at 12:22 PM, "Malea, Daniel" <daniel.malea at intel.com> wrote:
>
>> Hi all,
>>
>> I noticed MCJIT does not have any unit tests, so I added a few. If someone has a moment, could you please review the attached patches?
>>
>> Hopefully the unit tests can serve as an example of how to programatically use MCJIT for runtime code generation and execution.
>>
>> Patch descriptions:
>>
>> 000-move-memory-manager.patch: move LLIMCJITMemoryManager (from lli.cpp) to SectionMemoryManager in public headers (so we can test it)
>> 001-mcjit-tests-buildscripts.patch: CMake and Makefile changes required for this patchset
>> 002-section-memory-manager-tests.patch: Tests for the section-based memory manager
>> 003-mcjit-tests-impl.patch: Test cases for JITting empty modules, simple functions, function-calls, globals, and multiple-modules
>> 004-memory-buffer-tests.patch: Tests for MemoryBuffer (not really related to other patches, but MemoryBuffer had no tests and is used inside MCJIT)
>>
>> Not all test cases pass yet, due to either bugs or missing functionality (for example, multiple-module support) but I commented out such cases to keep "make check" happy.
>>
>> Also, I posted the combined patches on the LLVM reviewboard instance (for easier reading/reviewing): http://llvm.org/reviews/r/11/
>>
>> Thanks,
>> Dan
>>
>>
>> <000-move-memory-manager.patch><001-mcjit-tests-buildscripts.patch><002-section-memory-manager-tests.patch><003-mcjit-tests-impl.patch><004-memory-buffer-tests.patch>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 001-buildscripts-for-unittests.patch
Type: application/octet-stream
Size: 2967 bytes
Desc: 001-buildscripts-for-unittests.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121002/894ad2aa/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 002-mcjit-unittests.patch
Type: application/octet-stream
Size: 24840 bytes
Desc: 002-mcjit-unittests.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121002/894ad2aa/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 003-memorybuffer-unittests.patch
Type: application/octet-stream
Size: 3307 bytes
Desc: 003-memorybuffer-unittests.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121002/894ad2aa/attachment-0002.obj>