RFR(S): ZGC: C2: Oop instance cloning causing skipped compiles

[Nils Eliasson]

On 2019-11-28 16:47, Vladimir Ivanov wrote:
>
>> http://cr.openjdk.java.net/~neliasso/8234520/webrev.06/
>
> Looks good.
>
> src/hotspot/share/gc/z/c2/zBarrierSetC2.cpp
>
> +#include "opto/castnode.hpp"
>
> Unused?

Yes, I'll remove that one.

Thanks!

// Nils

>
> Best regards,
> Vladimir Ivanov
>
>>>>>> On 2019-11-21 12:53, Nils Eliasson wrote:
>>>>>>> I updated this to version 2.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~neliasso/8234520/webrev.02/
>>>>>>>
>>>>>>> I found a problen running 
>>>>>>> compiler/arguments/TestStressReflectiveCode.java
>>>>>>>
>>>>>>> Even though the clone was created as a oop clone, the type node 
>>>>>>> type returns isa_aryprt. This is caused by the src ptr not being 
>>>>>>> the base pointer. Until I fix that I wanted a more robust test.
>>>>>>>
>>>>>>> In this webrev I split up the is_clonebasic into is_clone_oop 
>>>>>>> and is_clone_array. (is_clone_oop_array is already there). 
>>>>>>> Having a complete set with the three clone types allows for a 
>>>>>>> robust test and easy verification. (The three variants end up in 
>>>>>>> different paths with different GCs).
>>>>>>>
>>>>>>> Regards,
>>>>>>>
>>>>>>> Nils
>>>>>>>
>>>>>>>
>>>>>>> On 2019-11-20 15:25, Nils Eliasson wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> I found a few bugs after the enabling of the clone intrinsic in 
>>>>>>>> ZGC.
>>>>>>>>
>>>>>>>> 1) The arraycopy clone_basic has the parameters adjusted to 
>>>>>>>> work as a memcopy. For an oop the src is pointing inside the 
>>>>>>>> oop to where we want to start copying. But when we want to do a 
>>>>>>>> runtime call to clone - the parameters are supposed to be the 
>>>>>>>> actual src oop and dst oop, and the size should be the instance 
>>>>>>>> size.
>>>>>>>>
>>>>>>>> For now I have made a workaround. What should be done later is 
>>>>>>>> using the offset in the arraycopy node to encode where the 
>>>>>>>> payload is, so that the base pointers are always correct. But 
>>>>>>>> that would require changes to the BarrierSet classes of all 
>>>>>>>> GCs. So I leave that for next release.
>>>>>>>>
>>>>>>>> 2) The size parameter of the TypeFunc for the runtime call has 
>>>>>>>> the wrong type. It was originally Long but missed the upper 
>>>>>>>> Half, it was fixed to INT (JDK-8233834), but that is wrong and 
>>>>>>>> causes the compiles to be skipped. We didn't notice that since 
>>>>>>>> they failed silently. That is also why we didn't notice problem 
>>>>>>>> #1 too.
>>>>>>>>
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8234520
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~neliasso/8234520/webrev.01/
>>>>>>>>
>>>>>>>> Please review!
>>>>>>>>
>>>>>>>> Nils
>>>>>>>>