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-gc-dev
mailing list