RFR: JDK-8223244: Fix usage of ARRAYCOPY_DISJOINT decorator

[David Holmes]

On 3/05/2019 8:46 pm, Roman Kennke wrote:
> Hi David,
> 
>> Not a review as I'm not familiar with the code, but I did take a look 
>> and have one comment ...
>>
>> On 3/05/2019 1:19 am, Roman Kennke wrote:
>>>   In stubGenerator_x86_64.cpp, the ARRAYCOPY_DISJOINT decorator is 
>>> missing in one place, and wrongly added in another. This should be 
>>> fixed, because Shenandoah will rely on correct DISJOINT decorator 
>>> soon, when we implement JDK-8222859.
>>>
>>> Specifically, the conjoint routine pretends to be disjoint. The 
>>> checkcast routine omits the disjoint decorator, even though it is 
>>> always disjoint (otherwise it wouldn't need checkcast).
>>
>> Can you add a comment somewhere documenting the fact that the 
>> checkcast_arraycopy is always disjoint, as in other cases "disjoint" 
>> forms part of the name.
> 
> Thanks. Done:
> 
> Incremental:
> http://cr.openjdk.java.net/~rkennke/JDK-8223244/webrev.01.diff/
> Full:
> http://cr.openjdk.java.net/~rkennke/JDK-8223244/webrev.01/

Something in ./share/runtime/stubRoutines.hpp would be nice too :)

Thanks,
David
------

> Roman
> 
> 
>> Thanks,
>> David
>> -----
>>
>>> Unfortunately, the fix is less trivial than it sounds. 
>>> ModRefBarrierSetAssembler's prologue implicitely assumes that 
>>> checkcast->!disjoint and gets the register shuffling wrong. Same for 
>>> ShenandoahBarrierSetAssembler. Hence the re-shuffling of that code. 
>>> It should be clearer/more explicit now, and equivalent to what we had 
>>> before.
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8223244
>>> Webrev:
>>> http://cr.openjdk.java.net/~rkennke/8223244/webrev.00/
>>>
>>> Testing: hotspot/tier1, hotspot_gc_shenandoah both x86 and aarch64. 
>>> Intend to do jdk-submit before push.
>>>
>>> Can I please get a review?
>>>
>>> Thanks,
>>> Roman