RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v5]

Nick Gasson ngasson at openjdk.java.net
Wed Oct 20 03:16:12 UTC 2021


On Wed, 20 Oct 2021 00:22:30 GMT, Paul Sandoz <psandoz at openjdk.org> wrote:

>> This PR improves the performance of vector operations that accept masks on architectures that support masking in hardware, specifically Intel AVX512 and ARM SVE.
>> 
>> On architectures that do not support masking in hardware the same technique as before is applied to most operations, specifically composition using blend.
>> 
>> Masked loads/stores are a special form of masked operation that require additional care to ensure out-of-bounds access throw exceptions. The range checking has not been fully optimized and will require further work.
>> 
>> No API enhancements were required and only a few additional tests were needed.
>
> Paul Sandoz has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits:
> 
>  - Merge branch 'master' into JDK-8271515-vector-api
>  - Resolve review comments.
>  - Merge branch 'master' into JDK-8271515-vector-api
>  - Apply patch from https://github.com/openjdk/panama-vector/pull/152
>  - Apply patch from https://github.com/openjdk/panama-vector/pull/142
>  - Apply patch from https://github.com/openjdk/panama-vector/pull/139
>  - Apply patch from https://github.com/openjdk/panama-vector/pull/151
>  - Add new files.
>  - 8271515: Integration of JEP 417: Vector API (Third Incubator)

AArch64 changes look ok apart from some minor comments.

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 2057:

> 2055: 
> 2056:   const int num_of_regs = PRegisterImpl::number_of_saved_registers;
> 2057:   unsigned char regs[num_of_regs];

Seems clearer to use `PRegisterImpl::number_of_saved_registers` directly rather than introducing `num_of_regs`.

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 2078:

> 2076: }
> 2077: 
> 2078: // Return the number of dwords poped

popped

src/hotspot/cpu/aarch64/register_aarch64.hpp line 248:

> 246:     // AArch64 has 8 governing predicate registers, but p7 is used as an
> 247:     // all-1s register so the predicates to save are from p0 to p6 if we
> 248:     // don't have non-governing predicate registers support.

This comment is a bit difficult to read. How about:


    // AArch64 has 8 governing predicate registers, but p7 is used as an
    // all-1s register so the predicates to save are from p0 to p6. We
    // don't support non-governing predicate registers.

src/hotspot/cpu/aarch64/vmreg_aarch64.hpp line 42:

> 40: 
> 41: inline Register as_Register() {
> 42:   assert(is_Register(), "must be");

I think it's better to leave this file as it was if you're only making whitespace changes here.

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

Marked as reviewed by ngasson (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5873


More information about the core-libs-dev mailing list