From 31f6d2e95b0340061d1960e74182f095a57637f3 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Tue, 3 Dec 2024 14:06:19 +0800 Subject: [PATCH 1/4] Fix compressed `val` register. In `MMTkBarrierSetAssembler`, `object_reference_write_post` is called after calling `BarrierSetAssembler::store_at` which modifies the `val` register and compresses its value in place. Consequently, `object_reference_write_post` will see a compressed pointer in the `val` register. We decompress the pointer in the slow path before calling into MMTk core. --- openjdk/barriers/mmtkObjectBarrier.cpp | 19 +++++++++++++++++-- openjdk/barriers/mmtkObjectBarrier.hpp | 2 +- openjdk/mmtkBarrierSetAssembler_x86.hpp | 7 +++++-- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/openjdk/barriers/mmtkObjectBarrier.cpp b/openjdk/barriers/mmtkObjectBarrier.cpp index d95944bc..2311f0f3 100644 --- a/openjdk/barriers/mmtkObjectBarrier.cpp +++ b/openjdk/barriers/mmtkObjectBarrier.cpp @@ -38,8 +38,11 @@ void MMTkObjectBarrierSetRuntime::object_reference_write_post(oop src, oop* slot #define __ masm-> -void MMTkObjectBarrierSetAssembler::object_reference_write_post(MacroAssembler* masm, DecoratorSet decorators, Address dst, Register val, Register tmp1, Register tmp2) const { +void MMTkObjectBarrierSetAssembler::object_reference_write_post(MacroAssembler* masm, DecoratorSet decorators, Address dst, Register val, Register tmp1, Register tmp2, bool compensate_val_reg) const { if (can_remove_barrier(decorators, val, /* skip_const_null */ true)) return; + + bool is_not_null = (decorators & IS_NOT_NULL) != 0; + Register obj = dst.base(); #if MMTK_ENABLE_BARRIER_FASTPATH Label done; @@ -70,7 +73,19 @@ void MMTkObjectBarrierSetAssembler::object_reference_write_post(MacroAssembler* __ movptr(c_rarg0, obj); __ lea(c_rarg1, dst); - __ movptr(c_rarg2, val == noreg ? (int32_t) NULL_WORD : val); + if (val == noreg) { + __ xorptr(c_rarg2, c_rarg2); + } else { + if (compensate_val_reg && UseCompressedOops) { + // `val` is compressed. Decompress it. + if (is_not_null) { + __ decode_heap_oop_not_null(val); + } else { + __ decode_heap_oop(val); + } + } + __ movptr(c_rarg2, val); + } __ call_VM_leaf_base(FN_ADDR(MMTkBarrierSetRuntime::object_reference_write_slow_call), 3); __ bind(done); diff --git a/openjdk/barriers/mmtkObjectBarrier.hpp b/openjdk/barriers/mmtkObjectBarrier.hpp index c6087dad..6baf7bae 100644 --- a/openjdk/barriers/mmtkObjectBarrier.hpp +++ b/openjdk/barriers/mmtkObjectBarrier.hpp @@ -30,7 +30,7 @@ class MMTkObjectBarrierSetRuntime: public MMTkBarrierSetRuntime { class MMTkObjectBarrierSetAssembler: public MMTkBarrierSetAssembler { protected: - virtual void object_reference_write_post(MacroAssembler* masm, DecoratorSet decorators, Address dst, Register val, Register tmp1, Register tmp2) const override; + virtual void object_reference_write_post(MacroAssembler* masm, DecoratorSet decorators, Address dst, Register val, Register tmp1, Register tmp2, bool compensate_val_reg) const override; public: virtual void arraycopy_prologue(MacroAssembler* masm, DecoratorSet decorators, BasicType type, Register src, Register dst, Register count) override; virtual void arraycopy_epilogue(MacroAssembler* masm, DecoratorSet decorators, BasicType type, Register src, Register dst, Register count) override; diff --git a/openjdk/mmtkBarrierSetAssembler_x86.hpp b/openjdk/mmtkBarrierSetAssembler_x86.hpp index 88a5f03e..383b688f 100644 --- a/openjdk/mmtkBarrierSetAssembler_x86.hpp +++ b/openjdk/mmtkBarrierSetAssembler_x86.hpp @@ -16,7 +16,8 @@ class MMTkBarrierSetAssembler: public BarrierSetAssembler { /// Full pre-barrier virtual void object_reference_write_pre(MacroAssembler* masm, DecoratorSet decorators, Address dst, Register val, Register tmp1, Register tmp2) const {} /// Full post-barrier - virtual void object_reference_write_post(MacroAssembler* masm, DecoratorSet decorators, Address dst, Register val, Register tmp1, Register tmp2) const {} + /// `compensate_val_reg` is true if this function is called after `BarrierSetAssembler::store_at` which compresses the pointer in the `val` register in place. + virtual void object_reference_write_post(MacroAssembler* masm, DecoratorSet decorators, Address dst, Register val, Register tmp1, Register tmp2, bool compensate_val_reg) const {} /// Barrier elision test virtual bool can_remove_barrier(DecoratorSet decorators, Register val, bool skip_const_null) const { @@ -34,7 +35,9 @@ class MMTkBarrierSetAssembler: public BarrierSetAssembler { virtual void store_at(MacroAssembler* masm, DecoratorSet decorators, BasicType type, Address dst, Register val, Register tmp1, Register tmp2) override { if (type == T_OBJECT || type == T_ARRAY) object_reference_write_pre(masm, decorators, dst, val, tmp1, tmp2); BarrierSetAssembler::store_at(masm, decorators, type, dst, val, tmp1, tmp2); - if (type == T_OBJECT || type == T_ARRAY) object_reference_write_post(masm, decorators, dst, val, tmp1, tmp2); + // BarrierSetAssembler::store_at modifies val and make it compressed if UseCompressedOops is true. + // We need to compensate for this change and decode it in object_reference_write_post. + if (type == T_OBJECT || type == T_ARRAY) object_reference_write_post(masm, decorators, dst, val, tmp1, tmp2, true); } /// Generate C1 write barrier slow-call stub From 0748d71dbb0bb3f33349aaed4f572b3a873b66f7 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Tue, 3 Dec 2024 14:13:38 +0800 Subject: [PATCH 2/4] Fix for no-fast-path code paths. --- openjdk/barriers/mmtkObjectBarrier.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/openjdk/barriers/mmtkObjectBarrier.cpp b/openjdk/barriers/mmtkObjectBarrier.cpp index 2311f0f3..f27cd5ef 100644 --- a/openjdk/barriers/mmtkObjectBarrier.cpp +++ b/openjdk/barriers/mmtkObjectBarrier.cpp @@ -70,6 +70,7 @@ void MMTkObjectBarrierSetAssembler::object_reference_write_post(MacroAssembler* __ andptr(tmp2, 1); __ cmpptr(tmp2, 1); __ jcc(Assembler::notEqual, done); +#endif __ movptr(c_rarg0, obj); __ lea(c_rarg1, dst); @@ -86,13 +87,11 @@ void MMTkObjectBarrierSetAssembler::object_reference_write_post(MacroAssembler* } __ movptr(c_rarg2, val); } - __ call_VM_leaf_base(FN_ADDR(MMTkBarrierSetRuntime::object_reference_write_slow_call), 3); +#if MMTK_ENABLE_BARRIER_FASTPATH + __ call_VM_leaf_base(FN_ADDR(MMTkBarrierSetRuntime::object_reference_write_slow_call), 3); __ bind(done); #else - __ movptr(c_rarg0, obj); - __ lea(c_rarg1, dst); - __ movptr(c_rarg2, val == noreg ? (int32_t) NULL_WORD : val); __ call_VM_leaf_base(FN_ADDR(MMTkBarrierSetRuntime::object_reference_write_post_call), 3); #endif } From 91f9da80c385ec4218491cf4e559af225545990b Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Tue, 3 Dec 2024 14:37:58 +0800 Subject: [PATCH 3/4] Add CI scripts --- .github/scripts/ci-test-minimal.sh | 2 ++ .github/scripts/ci-test-only-normal.sh | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/.github/scripts/ci-test-minimal.sh b/.github/scripts/ci-test-minimal.sh index 14b66e0a..373f9852 100755 --- a/.github/scripts/ci-test-minimal.sh +++ b/.github/scripts/ci-test-minimal.sh @@ -27,6 +27,8 @@ MMTK_PLAN=MarkSweep runbms_dacapo2006_with_heap_multiplier fop 8 MMTK_PLAN=NoGC runbms_dacapo2006_with_heap_size fop 1000 1000 # Test heap resizing MMTK_PLAN=GenImmix runbms_dacapo2006_with_heap_size fop 20 100 +# Test compressed oops with heap range > 4GB +MMTK_PLAN=GenImmix runbms_dacapo2006_with_heap_size fop 20 5000 # Test no compressed oop MMTK_PLAN=GenImmix runbms_dacapo2006_with_heap_multiplier fop 4 -XX:-UseCompressedOops -XX:-UseCompressedClassPointers diff --git a/.github/scripts/ci-test-only-normal.sh b/.github/scripts/ci-test-only-normal.sh index 8f834d1b..4d7dfd57 100755 --- a/.github/scripts/ci-test-only-normal.sh +++ b/.github/scripts/ci-test-only-normal.sh @@ -40,6 +40,10 @@ run_all 4 # Test heap resizing runbms_dacapo2006_with_heap_size fop 20 100 +# Test compressed oops with heap range > 4GB +# When the heap size is larger than 4GiB, compressed oops will be shifted by 3 bits. +runbms_dacapo2006_with_heap_size fop 20 5000 + # --- StickyImmix --- export MMTK_PLAN=StickyImmix From 065e8144b7a10e8d7379b29feb80e0eb2c6488f3 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Tue, 3 Dec 2024 16:45:03 +0800 Subject: [PATCH 4/4] Revert changes to ci-test-only-normal.sh --- .github/scripts/ci-test-only-normal.sh | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/scripts/ci-test-only-normal.sh b/.github/scripts/ci-test-only-normal.sh index 4d7dfd57..8f834d1b 100755 --- a/.github/scripts/ci-test-only-normal.sh +++ b/.github/scripts/ci-test-only-normal.sh @@ -40,10 +40,6 @@ run_all 4 # Test heap resizing runbms_dacapo2006_with_heap_size fop 20 100 -# Test compressed oops with heap range > 4GB -# When the heap size is larger than 4GiB, compressed oops will be shifted by 3 bits. -runbms_dacapo2006_with_heap_size fop 20 5000 - # --- StickyImmix --- export MMTK_PLAN=StickyImmix