Tests creation for AOT compilations infrastructure

[georgkrylov]

Tests creation for AOT generated code infrastructure

What we are trying to do:

This issue is to ask for help with reviewing our approach to tests creation for (Contribute AOT Relocation framework to OMR #4557). These tests will be to provide functional verification to the lines of code that is outlined later in the issue. Our original approach is to design a new test target, guarded by the compiler test and AOT test defines for the CMake build system.

How far we are:

We have replicated the test to set up the layout but no detailed tests were created as of yet.

How we are going to do it now:

The lines that we plan to test are available by the following links:

https://github.com/eclipse/omr/blob/edc455de0865c597c3f631dd6cd0a2afac349e12/compiler/runtime/OMRRelocationTarget.hpp#L61-L81
https://github.com/eclipse/omr/blob/edc455de0865c597c3f631dd6cd0a2afac349e12/compiler/runtime/OMRRelocationRuntime.hpp#L67
https://github.com/eclipse/omr/blob/edc455de0865c597c3f631dd6cd0a2afac349e12/compiler/runtime/OMRRelocationRecord.hpp#L89-L210
https://github.com/eclipse/omr/blob/edc455de0865c597c3f631dd6cd0a2afac349e12/compiler/runtime/OMRAOTRelocationRuntime.hpp#L42-L53
https://github.com/eclipse/omr/blob/edc455de0865c597c3f631dd6cd0a2afac349e12/compiler/env/OMRAOTMethodHeader.hpp#L48-L63
https://github.com/eclipse/omr/blob/edc455de0865c597c3f631dd6cd0a2afac349e12/compiler/env/OMRAOTAdapter.hpp#L69-L219
https://github.com/eclipse/omr/blob/edc455de0865c597c3f631dd6cd0a2afac349e12/compiler/env/AOTStorageInterface.hpp#L33
https://github.com/eclipse/omr/blob/edc455de0865c597c3f631dd6cd0a2afac349e12/compiler/env/AOTMethodHeader.hpp#L31-L40
https://github.com/eclipse/omr/blob/edc455de0865c597c3f631dd6cd0a2afac349e12/compiler/env/AOTAdapter.hpp#L31
https://github.com/eclipse/omr/blob/edc455de0865c597c3f631dd6cd0a2afac349e12/compiler/control/CompileMethod.cpp#L443-L449 
https://github.com/eclipse/omr/blob/edc455de0865c597c3f631dd6cd0a2afac349e12/compiler/compile/OMRCompilation.hpp#L332-L333 
https://github.com/eclipse/omr/blob/edc455de0865c597c3f631dd6cd0a2afac349e12/compiler/codegen/Relocation.cpp#L567-L569 
https://github.com/eclipse/omr/blob/edc455de0865c597c3f631dd6cd0a2afac349e12/compiler/codegen/Relocation.cpp#L547-L549 
https://github.com/eclipse/omr/blob/edc455de0865c597c3f631dd6cd0a2afac349e12/compiler/codegen/OMRCodeGenerator.cpp#L1953-L1965 
https://github.com/eclipse/omr/blob/edc455de0865c597c3f631dd6cd0a2afac349e12/compiler/codegen/OMRCodeGenerator.cpp#L240-L245

Also, since GitHub does not automatically relink the files in case there's a force-push, the hard-coded list of files is also supplied ${omr_SOURCE_DIR}/compiler/codegen/OMRCodeGenerator.cpp (lines 240-245 & 1953-1965) ${omr_SOURCE_DIR}/compiler/codegen/Relocation.cpp (lines 547-549 & 567-569) ${omr_SOURCE_DIR}/compiler/compile/OMRCompilation.hpp (lines 332-333) ${omr_SOURCE_DIR}/compiler/control/CompileMethod.cpp (lines 443-449) ${omr_SOURCE_DIR}/compiler/env/AOTAdapter.hpp (line 31) ${omr_SOURCE_DIR}/compiler/env/AOTMethodHeader.hpp (lines 31-40) ${omr_SOURCE_DIR}/compiler/env/AOTStorageInterface.hpp (line 33) ${omr_SOURCE_DIR}/compiler/env/OMRAOTAdapter.hpp (lines 69-219) ${omr_SOURCE_DIR}/compiler/env/OMRAOTMethodHeader.hpp (lines 48-63) ${omr_SOURCE_DIR}/compiler/runtime/OMRAOTRelocationRuntime.hpp (lines 42-53) ${omr_SOURCE_DIR}/compiler/runtime/OMRRelocationRuntime.hpp (line 67) ${omr_SOURCE_DIR}/compiler/runtime/OMRRelocationTarget.hpp (lines 61-81) ${omr_SOURCE_DIR}/compiler/runtime/OMRRelocationRecord.hpp (lines 89-210) ${omr_SOURCE_DIR}/compiler/x/codegen/OMRAheadOfTimeCompile.hpp (lines 53-57)

Approach

For some of the tests, the approach will be as simple as running the method and seeing if the data is there. For other tests, we have no idea yet. An example of a puzzling problem, is the codegen testing with creating OMRAheadOfTimeCompile object that will be referenced in case OMR compiler is built as a standalone project, not as part of OpenJ9, for example.

Request for help

Dear community, in particular @0xdaryl , @mstoodle , @dsouzai , do you have any suggestions on:

1. Do we need to create the 47th test target (currently there are 46 if the compiler tests are enabled), or shall we add our test to existing compiler tests?
2. Would you call the files list complete w.r.t #4557 ? Shall we add something more, or were we overzealous in selecting what needs to be done?
3. The assumption is we need to support both autoconf and cmake build systems, is that correct?
4. Can you please help with the design of the PR sequences for this issue? How can we group the tests? Shall it be a PR per tests for a dedicated folder?
5. How can we make this simpler for reviewing? We are absolutely willing to dedicate substantial amounts of effort to make the process less complicated for everyone.
6. Are there any existing tests in OpenJ9 for that, that can be lifted to OMR?
7. Are there any additional comments and requests?

Thank you in advance everyone, for your time and answers!

The text of this issue was compiled with the help from @kbeaton2.

[georgkrylov]

We believe we somehow figured "how" to do the testing and using the test framework: the test target should just act like a project that would consume Eclipse OMR by the means of extensible classes. This approach appeared to be working for us.

However, the questions on "what might be useful" to test and any suggestions on the PR simplification process are still very welcomed!

[georgkrylov]

Currently, our selected approach is to create an additional test target "aot_infra_testing" and copy a reduced version of the test compiler there, to use it. Is it a viable strategy? That creates quite a lot of code duplication, but does not interact with other test. Or shall our work go directly under compiler test? Can anyone comment on that please?

[0xdaryl]

The sections of code you highlight above to test seem reasonable to me. Basic unit tests for creating a respository, adding relocations, writing/reading data, and testing other parts of the internals could all be done with some "AOT infra" tests as you suggest. Some of the functions you listed above are private, so you'll have to find a way to work around that limitation.

For persisting and restoring actual methods and interacting with non-AOTed methods, I think your goal should be to build some of the AOT testing capabilities into either the existing Tril compiler tests or the test compiler, mainly to prevent duplication and leverage re-use of tests. The test compiler itself is redundant with Tril testing (and its features are slowly migrating to Tril) and I consider it to be deprecated. For that reason I suggest trying to focus on what you can accomplish with getting the AOT tests co-existing with Tril. That would be ideal, but it might not be straightforward to just drop in. Some thinking/re-architecting will be required, but I suggest you start down that path before considering another approach.

I would say only focus on a CMake implementation going forward.

[georgkrylov]

Thank you for your comments, @0xdaryl . We will try to extend the tril tests then.

[georgkrylov]

Following an "in-office" discussion on the subject, the tests were suggested to be placed alongside compiler tests, not tril tests. We will try this approach.

[kbeaton2-UNB3035]

What was tested: ${omr_SOURCE_DIR}/compiler/env/AOTMethodHeader.hpp #L31-L40 by AOTMethodHeaderTest <- Connection to OMRAOTMethodHeader ${omr_SOURCE_DIR}compiler/env/OMRAOTMethodHeader.hpp #L48-L63 by AOTMethodHeaderTest <- Create and then call the methods to test that the data travels through it correctly.

${omr_SOURCE_DIR}2/compiler/env/AOTStorageInterface.hpp #L33 by AOTStorageTest <- Connection to AOTStorageInterface

${omr_SOURCE_DIR}/compiler/control/CompileMethod.cpp #L443-L449 by AOTAdapterTest <- with a created AOTAdapter for aotAdatper and a connection to Compilation to overwrite the if statement method to return true to allow this code to run, the method highlighted can then be tested to see if it creates the correct MethodHeader. ${omr_SOURCE_DIR}/compiler/env/AOTAdapter.hpp #L31 by AOTAdapterTest <- Connection to AOTAdapter created in OMRAOTAdapter ${omr_SOURCE_DIR}/compiler/env/OMRAOTAdapter.hpp #L69-L219 by AOTAdapter <-Tested methods getMethodCode()/getRegisteredAOTMethodHeader() and createAndRegisterAOTMethodHeader()/registerAOTMethodHeader(), by calling them directly and calling them through a separate method during compilation, respectively. Inside testjit the AOTAdapter had to be created and connected, and in ResolvedMethod externalName has to be available and retrievable.

[kbeaton2-UNB3035]

The methods in: ${omr_SOURCE_DIR}/compiler/compile/OMRCompilation.hpp#L332-L333 are not able to be tested directly due to being unable to access the instance of Compilation.