RFR: 8302114: RISC-V: Several foreign jtreg tests fail with debug build after JDK-8301818

Gui Cao gcao at openjdk.org
Sun Feb 12 15:36:27 UTC 2023


On Thu, 9 Feb 2023 06:14:18 GMT, Feilong Jiang <fjiang 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)

LGTM, Thank you for fixing this, it was my mistake.

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

Marked as reviewed by gcao (Author).

PR: https://git.openjdk.org/jdk/pull/12481


More information about the hotspot-dev mailing list