Initialize/shutdown portlib in JitBuilder and Test Compiler #5798
Conversation
nbhuiyan
requested review from
0xdaryl,
charliegracie,
Leonardo2718,
mstoodle,
rwy7 and
youngar
as code owners
github-actions
bot
added
build / configure
cmake
comp:compiler
comp:jitbuilder
comp:test
labels
| @@ -298,6 +298,7 @@ function(create_omr_compiler_library) | |||
| target_link_libraries(${COMPILER_NAME} | |||
| PUBLIC | |||
| omr_base | |||
| ${OMR_PORT_LIB} | |||
There was a problem hiding this comment.
Curious where is this set exactly? Also @dnakamura any concerns?
There was a problem hiding this comment.
nbhuiyan
force-pushed
the
omr-jit-port
branch
from
e38f710
to
7e5c5ea
Compare
|
Have you by any chance measured the change in the size of one of the JitBuilder samples, say, when this change is included? I don't think it will make a huge difference but I would like to verify that intuition. |
|
@mstoodle, with this change, I saw about a 4% increase in the on-disk size of the JitBuilder samples. |
nbhuiyan
force-pushed
the
omr-jit-port
branch
2 times, most recently
from
0edb60d
to
d875ae3
Compare
This change ensures that Port library is initialized and shutdown when JitBuilder and testcompiler are initialized and shutdown. This paves the way for using port library features within OMR Compiler code and replace duplicated functionality through use of the port library API. Signed-off-by: Nazim Bhuiyan <nubhuiyan@ibm.com>
|
Revived this PR as a result of discussions in #4339. @0xdaryl, @r30shah FYI On x86, there is a failed test as a result of a check performed as part of |
You are right. Might it be time to remove the old vs new comparison? |
supports_feature_test should return whether feature checks using the old and the new API match. However, for some AVX512-related features, it returned whether a particular feature is supported instead, resulting in a failed assertion. Signed-off-by: Nazim Bhuiyan <nubhuiyan@ibm.com>
|
Removing the old vs. new comparison may be beyond the scope of this PR. I fixed the cases where we may get the incorrect results from |
This is one of the first steps towards enabling the use of the port lib in the OMR Compiler. The responsibilty of initializing and shutting down the port lib is assigned to the JitBuilder and Test Compiler initialization phases, as they do not currently run as part of a VM that uses the OMR Port Library.
In the future, JitBuilder and Test Compiler can have an added initialization API that would accept an initialized omrportlib that is handled as part of a larger VM or runtime init/shutdown should there be JitBuilder user that also uses the OMR Port Lib.
This flexible approach has the benefit of being able to rely on having the port library initialized in the OMR compiler and used throughout the component, while not requiring JitBuilder users to deal with the port library initialization/shutdown if they do not plan on using it in their project.
The port library is already built as a Compiler dependency. The initialized port lib provided to the CompilerEnv constructor is set in the
omrPortLibfield of the class, from where it can be accessed during the lifetime of the compilation. Currently, the only places using this field in the Compiler is for CPU feature detection, but a fallback mechanism is still in place if theomrPortLibmember is not set. These checks and fallback mechanisms can be eliminated after we can ensure all OMR Compiler users are initializing the compiler with an initialized port lib. OpenJ9 already provides the OMR Port Lib during the CompilerEnv initialization, see J9CompilerEnv.cpp.In draft state while I am yet to fix test failures in cases where JitBuilder is used in test cases that initialize thread and port libraries or initialize or shutdown the JIT more than once per
::testing::Testsubclass. Thread library is required to be initialized to initialize port library, and the current set up would cause test failures when thread library is initialized more than once per process. This PR is meant to begin discussions and for me to get some feedback about the approach being proposed here.Issue: #1644
Signed-off-by: Nazim Bhuiyan nubhuiyan@ibm.com