RFR: 8321021: RISC-V: C2 VectorUCastB2X [v3]

[Hamlin Li]

On Mon, 11 Mar 2024 07:42:07 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fix typo
>
> src/hotspot/cpu/riscv/riscv_v.ad line 3220:
> 
>> 3218:   ins_encode %{
>> 3219:     BasicType bt = Matcher::vector_element_basic_type(this);
>> 3220:     if (is_floating_point_type(bt)) {
> 
> Could `bt` (the vector element basic type) be floating point type for `VectorUCastB2X` node? I see our aarch64  counterpart has this assertion: `assert(bt == T_SHORT || bt == T_INT || bt == T_LONG, "must be");` [1]. Same question for `VectorUCastS2X` and `VectorUCastI2X` nodes.
> 
> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/aarch64/aarch64_vector.ad#L3752

Yeh, seems it's not, and vector api does not have this operations either.
Fixed, thanks for catching.

> src/hotspot/cpu/riscv/riscv_v.ad line 3397:
> 
>> 3395:   predicate(Matcher::vector_element_basic_type(n) == T_FLOAT);
>> 3396:   match(Set dst (VectorCastL2X src));
>> 3397:   effect(TEMP_DEF dst);
> 
> I see you added `TEMP_DEF dst` for some existing instructs like this one here. Do we really need it?
> I don't see such a need when reading the overlap constraints on vector operands from the RVV spec [1]:
> 
> 
> A destination vector register group can overlap a source vector register group only if one of the following holds:
> 
>     The destination EEW equals the source EEW.
> 
>     The destination EEW is smaller than the source EEW and the overlap is in the lowest-numbered part of the source register group (e.g., when LMUL=1, vnsrl.wi v0, v0, 3 is legal, but a destination of v1 is not).
> 
>     The destination EEW is greater than the source EEW, the source EMUL is at least 1, and the overlap is in the highest-numbered part of the destination register group (e.g., when LMUL=8, vzext.vf4 v0, v6 is legal, but a source of v0, v2, or v4 is not).
> 
> 
> [1] https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#sec-vec-operands

You're right, thanks for sharing the information.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18040#discussion_r1521844532
PR Review Comment: https://git.openjdk.org/jdk/pull/18040#discussion_r1521843212