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

[Malea, Daniel]

Hi Andy,

Thanks for the heads up. Here's the updated unit tests, this time with the proper "-elf" component appended to the triple on Windows; confirmed to work with MSVS2010.

Also, I added a macro to skip the tests under the same circumstances as the ExecutionEngine (non-unit) tests for MCJIT by duplicating some of the logic from test/ExecutionEngine/MCJIT/lit.local.cfg.

To ease the patching process, I combined the cmake/makefile/test code into a single patch this time.




Cheers,
Dan

On 2012-10-03, at 8:09 PM, Kaylor, Andrew wrote:

Hi Dan,

The MCJIT unit tests fail on Windows because it’s generating COFF object files.  You’ll need to conditionally set the environment string to ‘elf’ in the target triple somewhere.

You should probably also be conditionally skipping these tests under the same conditions that the MCJIT ExecutionEngine non-unit tests are skipped.

-Andy

From: Malea, Daniel
Sent: Tuesday, October 02, 2012 3:31 PM
To: Jim Grosbach
Cc: llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>; Kaylor, Andrew
Subject: Re: [llvm-commits] [PATCH] unit tests for MCJIT


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<mailto: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<mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: mcjit_unittests_combined.patch
Type: application/octet-stream
Size: 32730 bytes
Desc: mcjit_unittests_combined.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121004/f81153ca/attachment.obj>