RFR: 8302908: RISC-V: Support masked vector arithmetic instructions for Vector API [v12]

Yanhong Zhu yzhu at openjdk.org
Tue Apr 18 07:05:52 UTC 2023


On Tue, 18 Apr 2023 06:34:49 GMT, Dingli Zhang <dzhang at openjdk.org> wrote:

>> Hi @feilongjiang , thanks for the view!
>> 
>> I think maybe we don't need to distinguish whether it is unordered or not here at the moment. For example, the aarch64 greater(than) or less(than) comparison of sve_comapre[1] in the vmaskcmp call uses the GE/GT[2][3] in the Assembler condition to make the determination (instead of using the LE/LT flag bits, which would include unordered as well).
>> 
>> The most obvious difference in the results between the sequence of `isgreater()`[4] and the sequence of this patch is that `isgreater()` does not set the invalid operation exception flag (NV=1) if operands contain NaN(s). It seems that we would be better to use the sequence like `isgreater()`  for the case if operands contain NaN(s).
>> 
>> We will update PR later.
>> 
>> [1] https://github.com/openjdk/jdk/blob/2b81faeb3514060e6c8c950ef4e39e299c43199d/src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp#L1127-L1143
>> [2] https://developer.arm.com/documentation/ddi0596/2021-12/Shared-Pseudocode/Shared-Functions?lang=en#impl-shared.FPCompareGE.3
>> [3] https://developer.arm.com/documentation/ddi0596/2021-12/Shared-Pseudocode/Shared-Functions?lang=en#impl-shared.FPCompareGT.3
>> [4] https://github.com/riscv/riscv-v-spec/blob/b9afd6f5709fe3f91ce39bb83695bcfaa78eef94/v-spec.adoc?plain=1#L3700-L3706
>
> Fixed.

> Thanks for the review!
> 
> I think there may be no problem here. The foating-point compare instructions follow the semantics of the scalar floating-point compare instructions[1] in RVV. For all three instructions (FEQ.S, FLT.S, FLE.S), the result is 0 if either operand is NaN[2]. So when one of the operands is NaN, `BoolTest::ge`, `BoolTest::gt`, `BoolTest::le` and `BoolTest::lt` will all generate a 0 on the corresponding bit.
> 
> Also a jtreg test case[3] proves that our current logic is fine. `GTFloat512VectorTests` covers the case where the input is Nan. The test will pass properly and generate the following compilation log which contains `vmaskcmp_rvv`:
> 
> ```
> 1ac     B20: # out( B49 B21 ) <- in( B48 B19 )  Freq: 4188.06
> 1ac     vmaskcmp_rvv V0, V4, V5, #3
> 1b8     
> 1b8     MEMBAR-store-store #@membar_storestore
> 1bc     # checkcastPP of R11, #@checkCastPP
> 1bc     vstoremask V1, V0
> 1c8     addi  R7, R11, #16 # ptr, #@addP_reg_imm
> 1cc     spill R11 -> [sp, #104] # spill size = 64
> 1ce     storeV [R7], V1 # vector (rvv)
> 1d6     ld  R19, [R23, #264] # ptr, #@loadP
> 1da     ld  R7, [R23, #280] # ptr, #@loadP
> 1de     addi  R28, R19, #16 # ptr, #@addP_reg_imm
> 1e2     bgeu  R28, R7, B49 #@cmpP_branch  P=0.000100 C=-1.000000
> ```
> 
> [1] https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#1313-vector-floating-point-compare-instructions [2] https://github.com/riscv/riscv-isa-manual/releases/download/draft-20230131-c0b298a/riscv-spec.pdf [3] https://github.com/openjdk/jdk/tree/master/test/jdk/jdk/incubator/vector/Float512VectorTests.java

Thank you for your explanation.
Looks good to me.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/12682#discussion_r1169583928


More information about the hotspot-compiler-dev mailing list