RFR: 8260355: AArch64: deoptimization stub should save vector registers [v3]

[Vladimir Ivanov]

On Mon, 1 Feb 2021 09:36:05 GMT, Nick Gasson <ngasson at openjdk.org> wrote:

>> This is an AArch64 port of the fix for JDK-8256056 "Deoptimization stub
>> doesn't save vector registers on x86". The problem is that a vector
>> produced by the Vector API may be stored in a register when the deopt
>> blob is called. Because the deopt blob only stores the lower half of
>> vector registers, the full vector object cannot be rematerialized during
>> deoptimization. So the following will crash on AArch64 with current JDK:
>> 
>>   make test TEST="jdk/incubator/vector" \
>>     JTREG="VM_OPTIONS=-XX:+DeoptimizeALot -XX:DeoptimizeALotInterval=0"
>> 
>> The fix is to store the full vector registers by passing
>> save_vectors=true to save_live_registers() in the deopt blob. Because
>> save_live_registers() places the integer registers above the floating
>> registers in the stack frame, RegisterSaver::r0_offset_in_bytes() needs
>> to calculate the SP offset based on whether full vectors were saved, and
>> whether those vectors were NEON or SVE, rather than using a static
>> offset as it does currently.
>> 
>> The change to VectorSupport::allocate_vector_payload_helper() is
>> required because we only store the lowest VMReg slot in the oop map.
>> However unlike x86 the vector registers are always saved in a contiguous
>> region of memory, so we can calculate the address of each vector element
>> as an offset from the address of the first slot. X86 handles this in
>> RegisterMap::pd_location() but that won't work on AArch64 because with
>> SVE there isn't a unique VMReg corresponding to each four-byte physical
>> slot in the vector (there are always exactly eight logical VMRegs
>> regardless of the actual vector length).
>> 
>> Tested hotspot_all_no_apps and jdk_core.
>
> Nick Gasson has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Merge branch 'master' into 8260355
>  - Move SVE slot handling to RegisterMap::pd_location
>  - 8260355: AArch64: deoptimization stub should save vector registers
>    
>    This is an AArch64 port of the fix for JDK-8256056 "Deoptimization stub
>    doesn't save vector registers on x86". The problem is that a vector
>    produced by the Vector API may be stored in a register when the deopt
>    blob is called. Because the deopt blob only stores the lower half of
>    vector registers, the full vector object cannot be rematerialized during
>    deoptimization. So the following will crash on AArch64 with current JDK:
>    
>      make test TEST="jdk/incubator/vector" \
>        JTREG="VM_OPTIONS=-XX:+DeoptimizeALot -XX:DeoptimizeALotInterval=0"
>    
>    The fix is to store the full vector registers by passing
>    save_vectors=true to save_live_registers() in the deopt blob. Because
>    save_live_registers() places the integer registers above the floating
>    registers in the stack frame, RegisterSaver::r0_offset_in_bytes() needs
>    to calculate the SP offset based on whether full vectors were saved, and
>    whether those vectors were NEON or SVE, rather than using a static
>    offset as it does currently.
>    
>    The change to VectorSupport::allocate_vector_payload_helper() is
>    required because we only store the lowest VMReg slot in the oop map.
>    However unlike x86 the vector registers are always saved in a contiguous
>    region of memory, so we can calculate the address of each vector element
>    as an offset from the address of the first slot. X86 handles this in
>    RegisterMap::pd_location() but that won't work on AArch64 because with
>    SVE there isn't a unique VMReg corresponding to each four-byte physical
>    slot in the vector (there are always exactly eight logical VMRegs
>    regardless of the actual vector length).
>    
>    Tested hotspot_all_no_apps and jdk_core.

src/hotspot/share/prims/vectorSupport.cpp line 141:

> 139:       int off   = (i * elem_size) % VMRegImpl::stack_slot_size;
> 140: 
> 141:       address elem_addr = reg_map->location(vreg, vslot) + off;

Unrelated to your change: please, put the following comment: 
address elem_addr = reg_map->location(vreg, vslot) + off; // assumes little endian element order

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

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