RFR: RFR: Eliminate write-barrier assembly stub (part 2)

[Aleksey Shipilev]

On 03/12/2018 10:17 AM, Roman Kennke wrote:
> Am 12.03.2018 um 10:03 schrieb Aleksey Shipilev:
>> On 03/10/2018 12:15 PM, Roman Kennke wrote:
>>> Differential:
>>> http://cr.openjdk.java.net/~rkennke/eliminate_wb_stub-c2/webrev.02.diff/
>>> Full patch:
>>> http://cr.openjdk.java.net/~rkennke/eliminate_wb_stub-c2/webrev.02/
>>
>> Oh, I see why we cannot remove the stub: FPU spills.
>>
>> shenandoah_wb_C is getting called with CallLeafNoFPNode for a reason: it makes the compiler to
>> believe the stub is not using FPU/XMM registers, and thus it can spill registers to them. When the
>> actual barrier hits, our shenandoah_wb_C saves the FPU/XMM registers before calling to the runtime
>> (and that call would use XMM for, say, object copying). See:
>>   http://hg.openjdk.java.net/shenandoah/jdk/rev/842e412a3f86
> 
> But the actual stub now does nothing to C2. I.e. C2 compiled code would
> jump to the stub, the stub would shuffle registers, spill FPU and so on
> and then straigt call into runtime.

Yes, but from perspective of C2, the shenandoah_wb_C does not use XMM registers (it caller-saves all
of them), and so we can use FP spills around the barrier and its slowpath. This is *not* about the
asm stub we removed, see the current code:

    __ save_vector_registers();  <--- !!!!
    __ movptr(rdi, rax);
    __ call_VM_leaf(CAST_FROM_FN_PTR(address, ShenandoahBarrierSet::write_barrier_JRT), rdi);
    __ restore_vector_registers();   <--- !!!!

And this happens inside the stub, so we can just call shenandoah_wb_C with CallLeafNoFP, and
everything works out fine.

> However, I don't really understand the FPU spilling issue. In my mind,
> it *should* turn out something like:
> 
> if (evac-in-progress && in_cset(obj)) {
>   save_fpu_regs();
>   call_runtime_stub();
>   restore_fpu_regs();
> }

Maybe, but I would not testify how C2 tracks the register dependencies. The trouble is, how do we
communicate that both branches do not affect XMM registers? Doing CallLeafNoFP to wb_stub is
supposed to do that, I think.

> It's probably worth to inspect the generated assembly code to look for
> problems. Do you happen to remember which program was affected by the
> FPU spilling issue?

http://mail.openjdk.java.net/pipermail/shenandoah-dev/2017-December/004468.html

>> The patch is at least inconsistent here (should be Op_CallLeaf):
>>
>> 4242     if (n->Opcode() == Op_CallLeafNoFP && n->as_Call()->_entry_point ==
>> CAST_FROM_FN_PTR(address, ShenandoahBarrierSet::write_barrier_JRT)) {
>>
>> So, I really believe we should keep the stub intact, maybe clean it up a bit by collapsing (!c_abi)
>> branches, removing ShenandoahWriteBarrierCsetTestInIR, etc.
> 
> As said above, we'd have to re-instate the actual write-barrier in
> assembly to draw benefit from the FPU issue, no?

No, we don't.

And the patch is inconsistent, because you call with CallLeafNode:

3760   Node* call = new CallLeafNode(OptoRuntime::shenandoah_write_barrier_Type(),
CAST_FROM_FN_PTR(address, ShenandoahBarrierSet::write_barrier_JRT), "shenandoah_write_barrier",
TypeRawPtr::BOTTOM);

...but check for CallLeafNoFP!

4242     if (n->Opcode() == Op_CallLeafNoFP && n->as_Call()->_entry_point ==
CAST_FROM_FN_PTR(address, ShenandoahBarrierSet::write_barrier_JRT)) {

-Aleksey