Integrated: 8256757: Incorrect MachCallRuntimeNode::ret_addr_offset() for CallLeafNoFP on x86_32

[Aleksey Shipilev]

On Thu, 26 Nov 2020 12:22:31 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> Importance: fixes one of `tier1` tests, including the runs in GH Actions.
> 
> JDK-8254231 added new assert in `output.cpp`:
> 
>     assert(!is_mcall || (call_returns[block->_pre_order] <= (uint) current_offset))
> 
> Which verifies that the offset returned by MachCallNode::ret_addr_offset() (and sub-types) at matches the emitted code, to avoid potential conflicts between oop maps of different calls.
> 
> It caught the failure running `compiler/intrinsics/string/TestStringLatin1IndexOfChar.java` on Linux x86_32. I think that happened because the test forced lower SSE settings. But more tests fail if you force with lower SSE settings with e.g. `TEST_VM_OPTS=-XX:UseSSE=1`. The real issue is `MachCallRuntimeNode::ret_addr_offset()` computing the offset incorrectly for `CallLeafNoFP`: the match rule for it does not include `FFree_Float_Stack_All`. With `UseSSE < 2`, `FFree_Float_Stack_All` becomes non-zero.
> 
> See the definitions in `x86_32.ad`:
> 
>   enc_class FFree_Float_Stack_All %{    // Free_Float_Stack_All
>     MacroAssembler masm(&cbuf);
>     int start = masm.offset();
>     if (UseSSE >= 2) {
>       if (VerifyFPU) {
>         masm.verify_FPU(0, "must be empty in SSE2+ mode");
>       }
>     } else { // <---- taking this with UseSSE < 2
>       // External c_calling_convention expects the FPU stack to be 'clean'.
>       // Compiled code leaves it dirty.  Do cleanup now.
>       masm.empty_FPU_stack();  
>     }
>     if (sizeof_FFree_Float_Stack_All == -1) {
>       sizeof_FFree_Float_Stack_All = masm.offset() - start;
>     } else {
>       assert(masm.offset() - start == sizeof_FFree_Float_Stack_All, "wrong size");
>     }
>   %}
> 
> // Call runtime without safepoint
> instruct CallLeafDirect(method meth) %{
>   match(CallLeaf);
>   effect(USE meth);
> 
>   ins_cost(300);
>   format %{ "CALL_LEAF,runtime " %}
>   opcode(0xE8); /* E8 cd */
>   ins_encode( pre_call_resets,
>               FFree_Float_Stack_All,
>               Java_To_Runtime( meth ),
>               Verify_FPU_For_Leaf, post_call_FPU );
>   ins_pipe( pipe_slow );
> %}
> 
> instruct CallLeafNoFPDirect(method meth) %{
>   match(CallLeafNoFP);
>   effect(USE meth);
> 
>   ins_cost(300);
>   format %{ "CALL_LEAF_NOFP,runtime " %}
>   opcode(0xE8); /* E8 cd */
>   ins_encode(pre_call_resets, Java_To_Runtime(meth));
>   ins_pipe( pipe_slow );
> %}
> 
> This does not affect `x86_64`, AFAICS.
> 
> Additional testing:
>  - [x] A few known failing tests on Linux x86_32
>  - [x] Linux x86_32 `tier1`
>  - [x] Linux x86_32 `tier1` with `-XX:UseSSE=1`

This pull request has now been integrated.

Changeset: 9a468d85
Author:    Aleksey Shipilev <shade at openjdk.org>
URL:       https://git.openjdk.java.net/jdk/commit/9a468d85
Stats:     9 lines in 5 files changed: 7 ins; 0 del; 2 mod

8256757: Incorrect MachCallRuntimeNode::ret_addr_offset() for CallLeafNoFP on x86_32

Reviewed-by: jiefu, kvn

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

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