RFR: 8318219: RISC-V: C2 ExpandBits

Fei Yang fyang at openjdk.org
Wed Nov 15 04:02:34 UTC 2023


On Tue, 14 Nov 2023 15:37:24 GMT, Hamlin Li <mli at openjdk.org> wrote:

> Hi,
> Can you review the change to add intrinsic for ExpandBits for Long & Integer?
> Thanks!
> 
> related to: https://github.com/openjdk/jdk/pull/16481
> 
> ## Test
> ### jtreg
> test/jdk/java/lang/CompressExpand*.java
> test/hotspot/jtreg/compiler/intrinsics/TestBitShuffleOpers.java
> test/hotspot/jtreg/compiler/intrinsics/
> 
> ### gtest
> test/hotspot/gtest/opto/test_compress_expand_bits.cpp

src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1731:

> 1729:   vmv_v_i(v4, 0);
> 1730:   // convert the src data from bits to bytes.
> 1731:   vmerge_vim(v4, v4, 1); // v0 as implicit mask vector

Maybe 'mask register' instead of 'mask vector' to be consistent with code comment in `C2_MacroAssembler::compress_bits_v`?

src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1733:

> 1731:   vmerge_vim(v4, v4, 1); // v0 as implicit mask vector
> 1732:   vmv_v_i(v12, 0);
> 1733:   // reset the dst data(in bytes) to zero.

Seems this code comment applies for the preceding line? You might want to move it to before L1732.

src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1740:

> 1738:   vsetvli(x0, t0, Assembler::e8, lmul);
> 1739:   viota_m(v8, v0);
> 1740:   vrgather_vv(v12, v4, v8, VectorMask::v0_t); // v0 as implicit mask vector

Similar here. Maybe 'mask register' instead of 'mask vector' to be consistent?

test/hotspot/jtreg/compiler/intrinsics/TestBitShuffleOpers.java line 34:

> 32:  *             vm.cpu.features ~= ".*sse2.*")) |
> 33:  *            (os.arch=="aarch64" & vm.cpu.features ~= ".*svebitperm.*") |
> 34:  *            (os.arch=="riscv64" & vm.cpu.features ~= ".*v.*"))

I think the matching for riscv64 should be: `(os.arch=="riscv64" & vm.cpu.features ~= ".*v,.*"))` as described on [JBS ](https://bugs.openjdk.org/browse/JDK-8315652) and discussed at [1].

[1] https://github.com/openjdk/jdk/pull/15579#issuecomment-1709745846

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16658#discussion_r1393544879
PR Review Comment: https://git.openjdk.org/jdk/pull/16658#discussion_r1393513622
PR Review Comment: https://git.openjdk.org/jdk/pull/16658#discussion_r1393545415
PR Review Comment: https://git.openjdk.org/jdk/pull/16658#discussion_r1393529167


More information about the hotspot-compiler-dev mailing list