RFR: 8346832: runtime/CompressedOops/CompressedCPUSpecificClassSpaceReservation.java fails on RISC-V [v2]

[Thomas Stuefe]

On Thu, 26 Dec 2024 02:40:51 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Hi, please review this small change fixing a test failure.
>> 
>> This test first expects `tryReserveForUnscaled` and `tryReserveForZeroBased` from the process output. In addition, when test without CDS, it also expects `reserve_between (range [0x0000000800000000-0x0000100000000000)` which is not lower than zero-based limit. But the process output gives `reserve_between (range [0x0000000100000000-0x0000100000000000)`, which is not expected.
>> 
>> Previously, we have the code handling this case in `CompressedKlassPointers::reserve_address_space_for_compressed_classes`.
>> But I find that the code is gone after [JDK-8305895](https://bugs.openjdk.org/browse/JDK-8305895): Implement JEP 450: Compact Object Headers. This is the related change:
>> 
>> 
>> diff --git a/src/hotspot/cpu/riscv/compressedKlass_riscv.cpp b/src/hotspot/cpu/riscv/compressedKlass_riscv.cpp
>> index cffadb4189b..7c8d6b8f5bb 100644
>> --- a/src/hotspot/cpu/riscv/compressedKlass_riscv.cpp
>> +++ b/src/hotspot/cpu/riscv/compressedKlass_riscv.cpp
>> @@ -56,9 +56,9 @@ char* CompressedKlassPointers::reserve_address_space_for_compressed_classes(size
>>      result = reserve_address_space_for_zerobased_encoding(size, aslr);
>>    }
>> 
>> -  // Failing that, optimize for case (3) - a base with only bits set between [33-44)
>> +  // Failing that, optimize for case (3) - a base with only bits set between [32-44)
>>    if (result == nullptr) {
>> -    const uintptr_t from = nth_bit(32 + (optimize_for_zero_base ? LogKlassAlignmentInBytes : 0));
>> +    const uintptr_t from = nth_bit(32);
>>      constexpr uintptr_t to = nth_bit(44);
>>      constexpr size_t alignment = nth_bit(32);
>>      result = reserve_address_space_X(from, to, size, alignment, aslr);
>> 
>> 
>> It seems to me more reasonable to add the `optimize_for_zero_base check` back and replace the `LogKlassAlignmentInBytes` with
>> `CompressedKlassPointers::max_shift()`. max_shift() is used in `CompressedKlassPointers::reserve_address_space_for_zerobased_encoding` to calculate `zerobased_max` as well [1].
>> 
>> Testing: same test passes with fastdebug build on linux-riscv64 platform. Tagging @tstuefe 
>> 
>> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/oops/compressedKlass.cpp#L201
>
> Fei Yang has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Fix typo
>  - Review comment

LGTM. Thanks for fixing.

-------------

Marked as reviewed by stuefe (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22879#pullrequestreview-2524645487