Skip to content
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

Fix issue in folding the conditional in the duplicated loop #7155

Merged

Conversation

a7ehuo
Copy link
Contributor

@a7ehuo a7ehuo commented Oct 25, 2023

It is incorrect to both version asynch check and fold the conditional in the duplicated loop, or version write barrier and fold the conditional in the duplicated loop.

For asynch check, when the loops are unbiased, do not version asynch check so that the conditional in both loops can be folded away.

For write barriers, the determination of whether or not to version write barriers happens after the duplicated loop has already been added to FoldConditional. If the conditional in the duplicated loop will be folded, do not version write barrier.

Fixes eclipse-openj9/openj9#17249

@a7ehuo a7ehuo requested review from jdmpapin and removed request for vijaysun-omr October 25, 2023 13:26
@a7ehuo
Copy link
Contributor Author

a7ehuo commented Oct 25, 2023

@jdmpapin May I ask you to review this change? Thank you!
@hzongaro fyi

@jdmpapin
Copy link
Contributor

LGTM, but I've already been very involved with this fix, to the point that I've been listed as a co-author of the commit. (BTW, GitHub only recognizes the Co-authored-by tag when it's after the last blank line in the message)

So @vijaysun-omr would you mind reviewing this one?

@jdmpapin jdmpapin requested review from vijaysun-omr and removed request for jdmpapin October 25, 2023 14:46
@a7ehuo a7ehuo force-pushed the fix-loop-versioner-FoldConditional-pr branch from 36ca08b to 88c3544 Compare October 25, 2023 15:01
It is incorrect to both version asynch check and fold
the conditional in the duplicated loop, or version write
barrier and fold the conditional in the duplicated loop.

For asynch check, when the loops are unbiased, do not
version asynch check so that the conditional in both
loops can be folded away.

For write barriers, the determination of whether or not to
version write barriers happens after the duplicated loop has
already been added to `FoldConditional`. If the conditional
in the duplicated loop will be folded, do not version write
barrier.

Fixes eclipse-openj9/openj9#17249

Co-Authored-By: Devin Papineau <devin@ajdmp.ca>
Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>
@a7ehuo a7ehuo force-pushed the fix-loop-versioner-FoldConditional-pr branch from 88c3544 to 250f161 Compare October 25, 2023 15:01
@a7ehuo
Copy link
Contributor Author

a7ehuo commented Oct 25, 2023

(BTW, GitHub only recognizes the Co-authored-by tag when it's after the last blank line in the message)

250f161 is updated to have the correct format

@vijaysun-omr vijaysun-omr self-assigned this Oct 25, 2023
@vijaysun-omr
Copy link
Contributor

jenkins build all

@vijaysun-omr
Copy link
Contributor

jenkins build riscv

@a7ehuo
Copy link
Contributor Author

a7ehuo commented Oct 30, 2023

linux_riscv64_cross failure. The same riscv failure has seen before in other PR build test: #6886 (comment), #6735 (comment)

11:57:19  21: [----------] 15 tests from ThreadCreateTest
11:57:19  21: omrthread_attr_destroy(NULL) failed: retVal 14 (e)
11:57:19  21: omrthread_attr_destroy(&attr) failed: retVal 14 (e)
11:57:19  21: omrthread_attr_set_name(&attr, testname) unsupported: retVal 12 (c)
11:57:19  21: omrthread_attr_set_name(&attr, testname2) unsupported: retVal 12 (c)
11:57:19  21: omrthread_attr_set_name(&attr, NULL) unsupported: retVal 12 (c)
11:57:19  21: omrthread_create_ex(&handle, (attr? &attr : J9THREAD_ATTR_DEFAULT), 1, threadmain, &data) failed: retVal 6 (6) : errno 22 Invalid argument
11:57:19  21:   ignoring omrthread_create failure
11:57:19  21: omrthread_create_ex(&handle, (attr? &attr : J9THREAD_ATTR_DEFAULT), 1, threadmain, &data) failed: retVal 6 (6) : errno 22 Invalid argument
11:57:19  21:   ignoring omrthread_create failure
11:57:19  21: omrthread_attr_set_schedpolicy(&attr, omrthread_schedpolicy_LastEnum) failed: retVal 15 (f)
11:57:19  21: omrthread_attr_set_priority(&attr, -1) failed: retVal 15 (f)
11:57:19  21: omrthread_attr_set_priority(&attr, 90) failed: retVal 15 (f)
11:57:19  21: omrthread_attr_set_priority(&attr, 12) failed: retVal 15 (f)
11:57:19  21: [----------] 15 tests from ThreadCreateTest (31 ms total)
11:57:19  21: 
11:57:19  21: [----------] 8 tests from JoinTest
11:57:23  21: [----------] 8 tests from JoinTest (6013 ms total)
11:57:23  21: 
11:57:23  21: [----------] 1 test from KeyDestructorTest
11:57:23  21: [----------] 1 test from KeyDestructorTest (3 ms total)
11:57:23  21: 
11:57:23  21: [----------] 5 tests from LockedMonitorCountTest
11:57:23  21: [----------] 5 tests from LockedMonitorCountTest (4 ms total)
11:57:23  21: 
11:57:23  21: [----------] 7 tests from SanityTest
13:40:02  Cancelling nested steps due to timeout
13:40:02  Sending interrupt signal to process
13:40:14  Terminated
13:40:14  script returned exit code 143

@vijaysun-omr vijaysun-omr merged commit e2086cf into eclipse-omr:master Oct 30, 2023
17 of 18 checks passed
@a7ehuo a7ehuo deleted the fix-loop-versioner-FoldConditional-pr branch March 6, 2024 16:34
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.

JDK 8/11/17/18 - incorrect result from JIT compilation due to erroneous loop condition evaluation
3 participants