RFR: 8302114: RISC-V: Several foreign jtreg tests fail with debug build after JDK-8301818
Feilong Jiang
fjiang at openjdk.org
Mon Feb 13 01:22:26 UTC 2023
On Thu, 9 Feb 2023 06:30:57 GMT, Fei Yang <fyang at openjdk.org> wrote:
>> This problem only triggers when using debug build to run foreign jtreg tests.
>> [JDK-8301818](https://bugs.openjdk.org/browse/JDK-8301818) made following code changes in function generate_generic_copy [1]:
>>
>>
>> // At this point, it is known to be a typeArray (array_tag 0x3).
>> #ifdef ASSERT
>> {
>> BLOCK_COMMENT("assert primitive array {");
>> Label L;
>> - __ mvw(t1, Klass::_lh_array_tag_type_value << Klass::_lh_array_tag_shift);
>> + __ mv(t1, Klass::_lh_array_tag_type_value << Klass::_lh_array_tag_shift);
>> __ bge(lh, t1, L);
>> __ stop("must be a primitive array");
>> __ bind(L);
>> BLOCK_COMMENT("} assert primitive array done");
>> }
>> #endif
>>
>>
>> Notably, `Klass::_lh_array_tag_type_value` is of type `unsigned int`:
>>
>> static const unsigned int _lh_array_tag_type_value = 0Xffffffff;
>> static const int _lh_array_tag_shift = BitsPerInt - _lh_array_tag_bits;
>> static const int _lh_array_tag_bits = 2;
>>
>> So `Klass::_lh_array_tag_type_value << Klass::_lh_array_tag_shift` would be `0xc0000000` of type `unsigned int`.
>>
>> Previously, `mvw` function would sign-extend this value into 64-bit `0xffffffffc0000000` since we want to do 64-bit signed compare and branch with `bge` instruction next.
>>
>> template<typename T, ENABLE_IF(std::is_integral<T>::value)>
>> inline void mv(Register Rd, T o) { li(Rd, (int64_t)o); }
>>
>> inline void mvw(Register Rd, int32_t imm32) { mv(Rd, imm32); }
>>
>>
>> But this is not the case when changed to use `mv` with type `unsigned int`. So I think this changes the behaviour.
>> A simple fix would be adding an explicit `int32_t` type conversion here for this value, like:
>>
>> __ mv(t1, (int32_t)(Klass::_lh_array_tag_type_value << Klass::_lh_array_tag_shift));
>>
>>
>>
>> P.S.: We also checked other places where moving an immediate into registers and some type conversions were added/removed.
>>
>>
>> [1] https://github.com/openjdk/jdk/commit/c04a982eb47170f3c613617179fca012bb4d40ae
>>
>> Testing:
>> - [x] jdk_foreign all passed on Unmatched board (fastdebug build)
>> - [x] tier1-3 on Unmatched board (release build)
>
> Marked as reviewed by fyang (Reviewer).
@RealFYang @zifeihan -- Thanks!
-------------
PR: https://git.openjdk.org/jdk/pull/12481
More information about the hotspot-dev
mailing list