On Tue, 22 Oct 2024 16:22:20 GMT, Roman Kennke <[email protected]> wrote:
>> Roman Kennke has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Update copyright
>> - Avoid assert/endless-loop in JFR code
>
> @egahlin / @mgronlun could you please review the JFR parts of this PR? One
> change is for getting the right prototype header, the other is for avoiding
> an endless loop/assert in a corner case.
> @rkennke can you include this small update for s390x as well:
>
> ```diff
> diff --git a/src/hotspot/cpu/s390/c1_MacroAssembler_s390.cpp
> b/src/hotspot/cpu/s390/c1_MacroAssembler_s390.cpp
> index 0f7e5c9f457..476e3d5daa4 100644
> --- a/src/hotspot/cpu/s390/c1_MacroAssembler_s390.cpp
> +++ b/src/hotspot/cpu/s390/c1_MacroAssembler_s390.cpp
> @@ -174,8 +174,11 @@ void C1_MacroAssembler::try_allocate(
> void C1_MacroAssembler::initialize_header(Register obj, Register klass,
> Register len, Register Rzero, Register t1) {
> assert_different_registers(obj, klass, len, t1, Rzero);
> if (UseCompactObjectHeaders) {
> - z_lg(t1, Address(klass, in_bytes(Klass::prototype_header_offset())));
> - z_stg(t1, Address(obj, oopDesc::mark_offset_in_bytes()));
> + z_mvc(
> + Address(obj, oopDesc::mark_offset_in_bytes()), /* move to */
> + Address(klass, in_bytes(Klass::prototype_header_offset())), /*
> move from */
> + sizeof(markWord) /* how much to move */
> + );
> } else {
> load_const_optimized(t1, (intx)markWord::prototype().value());
> z_stg(t1, Address(obj, oopDesc::mark_offset_in_bytes()));
> diff --git a/src/hotspot/cpu/s390/c2_MacroAssembler_s390.cpp
> b/src/hotspot/cpu/s390/c2_MacroAssembler_s390.cpp
> index 378d5e4cfe1..c5713161bf9 100644
> --- a/src/hotspot/cpu/s390/c2_MacroAssembler_s390.cpp
> +++ b/src/hotspot/cpu/s390/c2_MacroAssembler_s390.cpp
> @@ -46,7 +46,7 @@ void
> C2_MacroAssembler::load_narrow_klass_compact_c2(Register dst, Address src)
> // The incoming address is pointing into obj-start +
> klass_offset_in_bytes. We need to extract
> // obj-start, so that we can load from the object's mark-word instead.
> z_lg(dst, src.plus_disp(-oopDesc::klass_offset_in_bytes()));
> - z_srlg(dst, dst, markWord::klass_shift); // TODO: could be z_sra
> + z_srlg(dst, dst, markWord::klass_shift);
> }
>
> //------------------------------------------------------
> diff --git a/src/hotspot/cpu/s390/templateTable_s390.cpp
> b/src/hotspot/cpu/s390/templateTable_s390.cpp
> index 3cb1aba810d..5b8f7a20478 100644
> --- a/src/hotspot/cpu/s390/templateTable_s390.cpp
> +++ b/src/hotspot/cpu/s390/templateTable_s390.cpp
> @@ -3980,8 +3980,11 @@ void TemplateTable::_new() {
> // Initialize object header only.
> __ bind(initialize_header);
> if (UseCompactObjectHeaders) {
> - __ z_lg(tmp, Address(iklass,
> in_bytes(Klass::prototype_header_offset())));
> - __ z_stg(tmp, Address(RallocatedObject,
> oopDesc::mark_offset_in_bytes()));
> + __ z_mvc(
> + Address(RallocatedObject, oopDesc::mark_offset_in_bytes()), //
> move to
> + Address(iklass, in_bytes(Klass::prototype_header_offset())), //
> move from
> + sizeof(markWord) // how much to move
> + );
> } else {
> __ store_const(Address(RallocatedObject,
> oopDesc::mark_offset_in_bytes()),
> (long) markWord::prototype().value());
> ```
Hi Amit,
sorry I only now get to reply to this, I have been traveling. What does the
change do? Is it critical? Would it be possible to fix it after I intergrated
the JEP? Because any change that I do now invalidates existing reviews, and
might delay integration, and we're already running pretty close to RDP1. If at
all possible, I would prefer to take it after I intergrated the JEP - we can
have fixes well after RDP1, but not new features. If you agree, then please
file a follow-up issue.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/20677#issuecomment-2457674486
