RFR: JDK-8231086: Shenandoah: Stronger invariant for object-arraycopy

[Roman Kennke]

Hi Aleksey,

>> Webrev:
>> https://bugs.openjdk.java.net/browse/JDK-8231086
> 
> The webrev seems to be actually here:
>    http://cr.openjdk.java.net/~rkennke/JDK-8231086/webrev.00/
> 
> *) src/hotspot/cpu/aarch64/gc/shenandoah/shenandoahBarrierSetAssembler_aarch64.cpp
> 
> Indenting is bad at L63-65 here:
> 
>    60       if (dest_uninitialized) {
>    61         __ tbz(rscratch2, ShenandoahHeap::HAS_FORWARDED_BITPOS, done);
>    62       } else {
>    63       __ mov(rscratch2, ShenandoahHeap::HAS_FORWARDED | ShenandoahHeap::MARKING);
>    64       __ tst(rscratch1, rscratch2);
>    65       __ br(Assembler::EQ, done);
>    66       }

Right, fixed!

> *) src/hotspot/cpu/x86/gc/shenandoah/shenandoahBarrierSetAssembler_x86.cpp
> 
> I think we can do this to simplify the code and expose x86_32 path early:
> 
> #ifdef _LP64
>    assert(src == rdi, "expected");
>    assert(dst == rsi, "expected");
>    assert(count == rdx, "expected");
>    if (UseCompressedOops) {
>      if (dest_uninitialized) {
>         ....call write_ref_array_pre_duinit_narrow_oop_entry
>       } else {
>         ... call write_ref_array_pre_narrow_oop_entry
>       }
>    } else
>   #endif
>    {
>      if (dest_uninitialized) {
>        ... call write_ref_array_pre_duinit_oop_entry
>      } else {
>        ... call write_ref_array_pre_oop_entry
>      }
>    }

Good point! Fixed.

> *) src/hotspot/share/gc/shenandoah/c2/shenandoahBarrierSetC2.cpp
> 
> Do these locals serve any purpose?

No. Probably left-over from somewhere during dev.

>    const char* copyfunc_name = "shenandoah_clone";
>    address     copyfunc_addr = CAST_FROM_FN_PTR(address, ShenandoahRuntime::shenandoah_clone_barrier);
>    const TypePtr* raw_adr_type = TypeRawPtr::BOTTOM;
>    const TypeFunc* call_type = ShenandoahBarrierSetC2::shenandoah_clone_barrier_Type();
>    Node* call = phase->make_leaf_call(ctrl, mem, call_type, copyfunc_addr, copyfunc_name,
> raw_adr_type, src, dest, length);
> 
> Seems to be cleaner to inline them:
> 
>    Node* call = phase->make_leaf_call(ctrl, mem,
>                    ShenandoahBarrierSetC2::shenandoah_clone_barrier_Type(),
>                    CAST_FROM_FN_PTR(address, ShenandoahRuntime::shenandoah_clone_barrier),
>                    "shenandoah_clone",
>                    TypeRawPtr::BOTTOM,
>                    src, dest, length);

Yeah, fixed.

> *) src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp
> 
> Having this one is a cute trick:
>     static inline oop cas_oop(oop n, narrowOop* addr, narrowOop c);

Right? :-)

Incremental diff:
http://cr.openjdk.java.net/~rkennke/JDK-8231086/webrev.01.diff/
Full:
http://cr.openjdk.java.net/~rkennke/JDK-8231086/webrev.01/

Ok?

Roman