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