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

[Roman Kennke]

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.

If we want to get back that FPU spilling advantage, then we ought to
revert the other related patch that removed the actual assembly
write-barrier.

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();
}

IIRC, what seemed to (sometimes?) happen was:

save_fpu_regs();
if (evac-in-progress && in_cset(obj)) {
  call_runtime_stub();
}
restore_fpu_regs();

or such. Which should be fixed, if it's the case, instead of worked
around with a nasty assembly stub.

All that said, I did run benchmarks over the weekend because I was
having the same thoughts, and couldn't see a regression. I also checked
benchmarks back when I did the actual write-barrier assembly removal and
haven't seen a regression.

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?

> 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?

Roman