-
Notifications
You must be signed in to change notification settings - Fork 6.1k
8367601: Remove held_monitor_count #27570
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
8367601: Remove held_monitor_count #27570
Conversation
|
👋 Welcome back fbredberg! A progress list of the required criteria for merging this PR into |
|
@fbredber This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 43 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
|
@TheRealMDoerr, @RealFYang |
Webrevs
|
|
Looks correct (PPC64 and shared code changes) and tier1 has passed. Would be nice to clean up unused temp registers diff --git a/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp b/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp
index 0bcc24a23bf..9fe7e1f22ff 100644
--- a/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp
+++ b/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp
@@ -1639,7 +1639,6 @@ static void fill_continuation_entry(MacroAssembler* masm, Register reg_cont_obj,
assert_different_registers(reg_cont_obj, reg_flags);
Register zero = R8_ARG6;
Register tmp2 = R9_ARG7;
- Register tmp3 = R10_ARG8;
DEBUG_ONLY(__ block_comment("fill {"));
#ifdef ASSERT
@@ -1678,7 +1677,6 @@ static void fill_continuation_entry(MacroAssembler* masm, Register reg_cont_obj,
static void continuation_enter_cleanup(MacroAssembler* masm) {
Register tmp1 = R8_ARG6;
Register tmp2 = R9_ARG7;
- Register tmp3 = R10_ARG8;
#ifdef ASSERT
__ block_comment("clean {");
@@ -1689,8 +1687,8 @@ static void continuation_enter_cleanup(MacroAssembler* masm) {
__ ld_ptr(tmp1, ContinuationEntry::parent_cont_fastpath_offset(), R1_SP);
__ st_ptr(tmp1, JavaThread::cont_fastpath_offset(), R16_thread);
- __ ld_ptr(tmp3, ContinuationEntry::parent_offset(), R1_SP);
- __ st_ptr(tmp3, JavaThread::cont_entry_offset(), R16_thread);
+ __ ld_ptr(tmp2, ContinuationEntry::parent_offset(), R1_SP);
+ __ st_ptr(tmp2, JavaThread::cont_entry_offset(), R16_thread);
DEBUG_ONLY(__ block_comment("} clean"));
}
Thanks for doing it for all platforms! |
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.
RISC-V part of the change seems fine to me. My local hs:tier1-hs:tier3 passed with fastdebug build.
|
@fbredber |
Fixed those.
The same ways as eating different types of food enriches your life, so does programming for different CPUs. :) |
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.
PPC64 and shared code changes look good. Thanks!
|
I guess you want to update the Copyright headers eventually. |
Think I updated all the years to 2025. Did you see something I missed? |
I thought I had seen 2024 somewhere. But, I can't find it again. I guess I had looked at the wrong file. Your Copyright updates look fine. Sorry. |
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.
Looks good to me, thanks!
|
Thank you all for the reviews. Now let's... /integrate |
|
Going to push as commit e6781fd.
Your commit was automatically rebased without conflicts. |
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.
Good cleanup, thanks!
Since we have removed all other locking modes than lightweight locking (see: JDK-8344261), we no longer need:
_held_monitor_count_parent_held_monitor_count_jni_monitor_countThis PR removes them from shared code as well as from
X86,AArch64,PowerPCandRISC-V.They are not present in other platforms.
Tested tier1-7 (on supported platforms) without seeing any problems that can be traced to this code change.
PowerPCandRISC-Vhas been sanity checked using QEMU.Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27570/head:pull/27570$ git checkout pull/27570Update a local copy of the PR:
$ git checkout pull/27570$ git pull https://git.openjdk.org/jdk.git pull/27570/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 27570View PR using the GUI difftool:
$ git pr show -t 27570Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27570.diff
Using Webrev
Link to Webrev Comment