On Mon, 24 Aug 2026 01:37:39 GMT, Fei Yang <[email protected]> wrote:

>> Hi, please consider.
>> 
>> This is a follow-up to 
>> [JDK-8390101](https://bugs.openjdk.org/browse/JDK-8390101).
>> 
>> On RISC-V, several VectorMask logical operations involving an all-bits-set 
>> mask are currently emitted as sequences of multiple RVV mask instructions. 
>> These include:
>> 
>> - VectorMask.not()
>> - VectorMask.and(...).not()
>> - VectorMask.or(...).not()
>> - VectorMask.xor(...).not()
>> - VectorMask.eq()
>> - VectorMask.or(other.not())
>> 
>> RVV provides mask logical instructions that can implement these operations 
>> directly.
>> 
>> This change adds RISC-V C2 match rules to emit:
>> 
>> - vmnot.m for mask NOT
>> - vmnand.mm for mask NAND
>> - vmnor.mm for mask NOR
>> - vmxnor.mm for mask XNOR and mask equality
>> - vmorn.mm for mask OR-NOT
>> 
>> The rules cover byte, short, int, and long vector element types and replace 
>> multi-instruction mask logical sequences with a single RVV mask instruction.
>> 
>> ## Testing
>> 
>> `MaskLogicOperationsBenchmark` on sg2044:
>> 
>> 
>>                                                                              
>>       Before                 After
>> Benchmark                                        (size)   Mode  Cnt      
>> Score      Error      Score      Error   Units
>> MaskLogicOperationsBenchmark.byteMaskEq             256  thrpt   10   
>> 9018.332 ±   80.876  12940.779 ± 1240.139  ops/ms
>> MaskLogicOperationsBenchmark.byteMaskEq             512  thrpt   10   
>> 4542.277 ±  474.059   6833.785 ±  852.424  ops/ms
>> MaskLogicOperationsBenchmark.byteMaskEq            1024  thrpt   10   
>> 2485.999 ±   23.447   3783.676 ±   37.133  ops/ms
>> MaskLogicOperationsBenchmark.byteMaskNand           256  thrpt   10   
>> 9239.545 ±   76.134  13712.414 ±   87.500  ops/ms
>> MaskLogicOperationsBenchmark.byteMaskNand           512  thrpt   10   
>> 4865.590 ±   33.147   7037.323 ±  138.414  ops/ms
>> MaskLogicOperationsBenchmark.byteMaskNand          1024  thrpt   10   
>> 2498.022 ±   32.932   3757.491 ±   68.019  ops/ms
>> MaskLogicOperationsBenchmark.byteMaskNor            256  thrpt   10   
>> 9234.898 ±   60.028  13362.605 ±  803.210  ops/ms
>> MaskLogicOperationsBenchmark.byteMaskNor            512  thrpt   10   
>> 4564.248 ±  389.177   7175.499 ±  183.598  ops/ms
>> MaskLogicOperationsBenchmark.byteMaskNor           1024  thrpt   10   
>> 2391.446 ±  128.692   3801.884 ±   61.826  ops/ms
>> MaskLogicOperationsBenchmark.byteMaskNot            256  thrpt   10  
>> 12136.710 ±   42.704  12737.138 ±  583.812  ops/ms
>> MaskLogicOperationsBenchmark.byteMaskNot            512  thrpt   10   ...
>
> src/hotspot/cpu/riscv/riscv_v.ad line 4642:
> 
>> 4640: instruct vmask_xnorI(vRegMask dst, vRegMask src1, vRegMask src2, 
>> immI_M1 m1) %{
>> 4641:   match(Set dst (XorVMask (XorVMask src1 src2) (MaskAll m1)));
>> 4642:   match(Set dst (XorVMask src1 (XorVMask src2 (MaskAll m1))));
> 
> Question: There are two matchings for this instruct. But seems the newly-add 
> IR test only covered one of them, right? I am wondering if we really need 
> both of them.

Thanks for checking. Both matches are intentional and are exercised by 
different tests.
The first pattern:

    (XorVMask (XorVMask src1 src2) (MaskAll m1))

corresponds to `avm.xor(bvm).not()` and is covered by `testMaskXnorI`.

The second pattern:

    (XorVMask src1 (XorVMask src2 (MaskAll m1)))

corresponds to `avm.eq(bvm)` and is covered by `testMaskEqI`. 
`AbstractMask::eq()` is implemented as `return xor(m.not());`, which produces 
this second shape.

C2 does not currently reassociate these two `XorVMask` shapes, so both match 
patterns are needed.

> src/hotspot/cpu/riscv/riscv_v.ad line 4656:
> 
>> 4654: instruct vmask_xnorL(vRegMask dst, vRegMask src1, vRegMask src2, 
>> immL_M1 m1) %{
>> 4655:   match(Set dst (XorVMask (XorVMask src1 src2) (MaskAll m1)));
>> 4656:   match(Set dst (XorVMask src1 (XorVMask src2 (MaskAll m1))));
> 
> Same question here.

The same applies to the L variant: `testMaskXnorL` covers the first pattern, 
while `testMaskEqL` covers the second one.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/32480#discussion_r3841373201
PR Review Comment: https://git.openjdk.org/jdk/pull/32480#discussion_r3841377030

Reply via email to