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

[Jim Grosbach]

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