Skip to content

Move willBeEvaluatedAsCallByCodeGen from OMR - #20469

Merged
hzongaro merged 1 commit into
eclipse-openj9:masterfrom
IBMJimmyk:moveMethod
Nov 5, 2024
Merged

Move willBeEvaluatedAsCallByCodeGen from OMR#20469
hzongaro merged 1 commit into
eclipse-openj9:masterfrom
IBMJimmyk:moveMethod

Conversation

@IBMJimmyk

Copy link
Copy Markdown
Contributor

The OMR willBeEvaluatedAsCallByCodeGen implementation for x is almost entirely guarded by the J9_PROJECT_SPECIFIC macro. If the macro is not set, it always returns true which is what the default implementation already does. Since this implementation is only meaningful to OpenJ9, it is being moved here.

willNotInlineCompareAndSwapNative is only called from this implementation of willBeEvaluatedAsCallByCodeGen so it is being moved as well.

The OMR willBeEvaluatedAsCallByCodeGen implementation for x is almost
entirely guarded by the J9_PROJECT_SPECIFIC macro. If the macro is not
set, it always returns true which is what the default implementation
already does. Since this implementation is only meaningful to OpenJ9, it
is being moved here.

willNotInlineCompareAndSwapNative is only called from this
implementation of willBeEvaluatedAsCallByCodeGen so it is being moved as
well.

Signed-off-by: jimmyk <jimmyk@ca.ibm.com>
@IBMJimmyk

Copy link
Copy Markdown
Contributor Author

@hzongaro When you have some time, could you look at this as well?

@hzongaro hzongaro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these changes look good. Thanks!

@hzongaro

hzongaro commented Nov 1, 2024

Copy link
Copy Markdown
Member

I'll run testing both with and without eclipse-omr/omr#7516

Jenkins test sanity.functional,sanity.openjdk xlinux,xmac,win jdk8,jdk17,jdk21

@hzongaro

hzongaro commented Nov 1, 2024

Copy link
Copy Markdown
Member

Jenkins test sanity.functional,sanity.openjdk win32 jdk8

@IBMJimmyk

Copy link
Copy Markdown
Contributor Author

Mac failures seem to be due to an unreachable machine.
https://openj9-jenkins.osuosl.org/job/Test_openjdk8_j9_sanity.functional_x86-64_mac_Personal/229/console

10:45:08  update to openj9 sha: origin/pr/20469/merge
10:45:08  git fetch -q --unshallow
10:45:20  error: pathspec 'origin/pr/20469/merge' did not match any file(s) known to git
10:45:20  SHA not yet found. Continue fetching PR refs and tags...
10:45:20  git fetch -q --tags https://github.com/eclipse-openj9/openj9.git +refs/pull/*:refs/remotes/origin/pr/*
10:45:40  Cannot contact mac10-x86-2: java.lang.InterruptedException
10:46:27  error: 6267 bytes of body are still expected
10:46:27  fetch-pack: unexpected disconnect while reading sideband packet
10:46:27  fatal: early EOF
10:46:27  fatal: fetch-pack: invalid index-pack output

https://openj9-jenkins.osuosl.org/job/Test_openjdk17_j9_sanity.functional_x86-64_mac_Personal_testList_0/347/console

10:21:34  update to openj9 sha: origin/pr/20469/merge
10:21:34  git fetch -q --unshallow
10:21:45  error: pathspec 'origin/pr/20469/merge' did not match any file(s) known to git
10:21:45  SHA not yet found. Continue fetching PR refs and tags...
10:21:45  git fetch -q --tags https://github.com/eclipse-openj9/openj9.git +refs/pull/*:refs/remotes/origin/pr/*
10:22:12  Cannot contact mac10-x86-2: java.lang.InterruptedException
10:22:19  error: 824 bytes of body are still expected
10:22:19  fetch-pack: unexpected disconnect while reading sideband packet
10:22:19  fatal: early EOF
10:22:19  fatal: fetch-pack: invalid index-pack output

@hzongaro

hzongaro commented Nov 1, 2024

Copy link
Copy Markdown
Member

JDK 21 sanity.openjdk x86-64 Linux failures in jdk_security1_0 and jdk_security1_1 do not appear to be related to this change, although I can't find any known issues with similar symptoms. I'll keep an eye on this for now to see whether the same problem shows up in other test runs.

JDK 8 sanity.functional x86-64 Linux failure in testJitserverArguments_0 appears to be due to known issue #18599.

I will rerun the JDK 8 and 17 xmac sanity.functional testing.

Jenkins test sanity.functional xmac jdk8,jdk17

@hzongaro

hzongaro commented Nov 4, 2024

Copy link
Copy Markdown
Member

I think I've figured out the reason for the JDK 21 sanity.openjdk failures for x86-64 Linux and Windows. The errors reported are of the form:

11:04:39  test Versions.Test("src/java.xml.crypto/share/classes/org/jcp/xml/dsig/internal/dom/XMLDSigRI.java", java.util.regex.Pattern@d101e565, "src/java.xml.crypto/share/legal/santuario.md", ## Apache Santuario v(?<n>\S+), "java.xml.crypto/santuario.md"): success
11:04:39  test Versions.Test("src/java.base/share/data/publicsuffixlist/VERSION", java.util.regex.Pattern@93327084, "src/java.base/share/legal/public_suffix.md", java.util.regex.Pattern@7ce5978a, "java.base/public_suffix.md"): failure
11:04:39  java.lang.AssertionError: expected [-1] but found [629]

The problem appears to be that the SHA of the extension repo. used in the builds was 629642af80326f988264f20c40c4e8d78ea8b876 while that used during testing was e7f09c477fb071e2a67c135e3921c592a09db803. This looks like another instance of the problem previously reported in issue #20121.

I will move on to running testing with the OMR pull request.

Jenkins test sanity.functional,sanity.openjdk xlinux,xmac,win jdk8,jdk17,jdk21 depends eclipse-omr/omr#7516

@hzongaro

hzongaro commented Nov 4, 2024

Copy link
Copy Markdown
Member

Jenkins test sanity.functional,sanity.openjdk win32 jdk8 depends eclipse-omr/omr#7516

@hzongaro

hzongaro commented Nov 4, 2024

Copy link
Copy Markdown
Member

xmac builds failed with this error:

10:39:16  [ 99%] Building CXX object runtime/compiler/CMakeFiles/j9jit.dir/Users/jenkins/workspace/Build_JDK17_x86-64_mac_Personal/omr/compiler/x/codegen/FPCompareAnalyser.cpp.o
10:39:16  /Users/jenkins/workspace/Build_JDK17_x86-64_mac_Personal/omr/compiler/x/codegen/DataSnippet.cpp:106:71: error: shifting a negative signed value is undefined [-Werror,-Wshift-negative-value]
10:39:16        cursor = (uint8_t *)(((intptr_t)(cursor + 15)) & ((intptr_t)(-1)<<4));
10:39:16                                                          ~~~~~~~~~~~~~~^
10:39:16  1 error generated.

That problem was fixed by eclipse-omr/omr#7521. Rerunning those tests.

Jenkins test sanity.functional,sanity.openjdk xlinux,xmac,win jdk8,jdk17,jdk21 depends eclipse-omr/omr#7516

@hzongaro

hzongaro commented Nov 4, 2024

Copy link
Copy Markdown
Member

JDK 8 sanity.functional x86-64 Linux failure running testJitserverArguments_0 appears to be due to known issue #18599, as before.

@hzongaro

hzongaro commented Nov 4, 2024

Copy link
Copy Markdown
Member

Sigh! I inadvertently reran all tests in my earlier comment rather than just the xmac tests. My apologies for the churn and extra delay.

@hzongaro

hzongaro commented Nov 5, 2024

Copy link
Copy Markdown
Member

@hzongaro
hzongaro merged commit 9377d3f into eclipse-openj9:master Nov 5, 2024
@IBMJimmyk
IBMJimmyk deleted the moveMethod branch November 11, 2025 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants