-
Notifications
You must be signed in to change notification settings - Fork 706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor CompilationStrategy and CompilationController #16700
Conversation
ffeb6a8
to
3ecd030
Compare
I believe this requires a coordinated merge with eclipse/omr#6884 |
04f02d4
to
cb7c7c0
Compare
c45e31c
to
f0e396f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
f0e396f
to
d56c336
Compare
Jenkins test sanity all jdk8,jdk11,jdk17 depends eclipse/omr#6884 |
@jamesgua : there are some build failures on Windows. For example, https://openj9-jenkins.osuosl.org/job/Build_JDK8_x86-64_windows_Personal/841/consoleText
|
Update in OMR - TR::compilation corrected to TR::Compilation, pls rerun the build. Thx! |
Jenkins test sanity all jdk8,jdk11,jdk17 depends eclipse/omr#6884 |
1 similar comment
Jenkins test sanity all jdk8,jdk11,jdk17 depends eclipse/omr#6884 |
Jenkins test sanity zlinux jdk11 depends eclipse/omr#6884 |
Jenkins test sanity,sanity.openjdk all jdk8,jdk11,jdk17 depends eclipse/omr#6884 |
Jenkins test sanity.functional plinux jdk8,jdk11,jdk17 |
Jenkins test sanity.openjdk plinux jdk8,jdk11 |
Lone Z failure appears to be #12582 |
Jenkins test sanity.functional plinux jdk8,jdk11,jdk17 depends eclipse/omr#6884 |
Jenkins test sanity.openjdk plinux jdk8,jdk11 depends eclipse/omr#6884 |
Jenkins test sanity.functional plinux jdk17 depends eclipse/omr#6884 Looks like an infra issue. Retrying. |
@@ -380,6 +379,7 @@ JIT_PRODUCT_SOURCE_FILES+=\ | |||
compiler/runtime/ValueProfiler.cpp \ | |||
compiler/runtime/codertinit.cpp \ | |||
compiler/runtime/emfloat.c \ | |||
omr/compiler/control/CompilationController.cpp \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, just discovered this. You will also need to add the new file compiler/control/J9CompilationStrategy.cpp
to this file.
As this does not affect builds in the open project I don't need to re-run all the tests and will just re-run a private build instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change updated, pls take another look.
using CompilationController directly from OMR making CompilationStrategy extensible class rename DefaultCompilationStrategy to CompilationStrategy use self method to dereference member method Signed-off-by: Tao Guan <jamesgua@ca.ibm.com>
d56c336
to
fb9b96a
Compare
For the record, all previous acceptance testing run on this PR with its OMR dependency was successful. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UMA build was successful.
Signed-off-by: Tao Guan james_mango@yahoo.com