RFR: JDK-8223244: Fix usage of ARRAYCOPY_DISJOINT decorator

Roman Kennke rkennke at redhat.com
Fri May 3 14:40:14 UTC 2019


>>> 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 :)

Right. Added a comment there, and also checked and fixed x86_32:
http://cr.openjdk.java.net/~rkennke/JDK-8223244/webrev.02/

Thanks for 'reviewing'!
Roman



> 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


More information about the hotspot-runtime-dev mailing list