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