RFR: 8198949: Modularize arraycopy stub routine GC barriers

[Erik Österlund]

Hi Per,

Thanks for the review.

/Erik

On 2018-03-21 15:05, Per Liden wrote:
> Awesome! Looks good!
>
> /Per
>
> On 03/21/2018 02:58 PM, Erik Österlund wrote:
>> Hi Per,
>>
>> Thank you for the review.
>>
>> On 2018-03-21 14:36, Per Liden wrote:
>>> Looks good!
>>>
>>> I have one small-ish suggestion/question. The some of the headers in 
>>> hotspot/share/gc/shared/ does:
>>>
>>> 30 #ifndef ZERO
>>> 31 #include CPU_HEADER(gc/shared/barrierSetAssembler)
>>> 32 #endif
>>> 33
>>> 34 class BarrierSetAssembler;
>>>
>>> Shouldn't we just have CPU-specific files for zero too, which would 
>>> just do the forward declaration? That would remove platform specific 
>>> #ifndef's in the shared code, which is always nice.
>>
>> Sure.
>>
>> New full webrev:
>> http://cr.openjdk.java.net/~eosterlund/8198949/webrev.04/
>>
>> Incremental webrev:
>> http://cr.openjdk.java.net/~eosterlund/8198949/webrev.03_04/
>>
>> Thanks,
>> /Erik
>>
>>>
>>> /Per
>>>
>>> On 03/19/2018 12:49 PM, Erik Österlund wrote:
>>>> Hi,
>>>>
>>>> After some internal discussions, it turns out that the name 
>>>> "*BSCodeGen" was not so popular, and has been changed to 
>>>> *BarrierSetAssembler instead.
>>>> I have rebased this on top of 8199604: Rename CardTableModRefBS to 
>>>> CardTableBarrierSet and went ahead with the performing the required 
>>>> renaming.
>>>>
>>>> New full webrev:
>>>> http://cr.openjdk.java.net/~eosterlund/8198949/webrev.03/
>>>>
>>>> New incremental webrev (from the rebased version):
>>>> http://cr.openjdk.java.net/~eosterlund/8198949/webrev.02_03/
>>>>
>>>> Thanks,
>>>> /Erik
>>>>
>>>> On 2018-03-13 10:47, Erik Österlund wrote:
>>>>> Hi Roman,
>>>>>
>>>>> Thanks for the review.
>>>>>
>>>>> /Erik
>>>>>
>>>>> On 2018-03-13 10:26, Roman Kennke wrote:
>>>>>> Am 09.03.2018 um 17:58 schrieb Erik Österlund:
>>>>>>> Hi,
>>>>>>>
>>>>>>> The GC barriers for arraycopy stub routines are not as modular 
>>>>>>> as they
>>>>>>> could be. They currently use switch statements to check which GC 
>>>>>>> barrier
>>>>>>> set is being used, and call one or another barrier based on 
>>>>>>> that, with
>>>>>>> registers already allocated in such a way that it can only be 
>>>>>>> used for
>>>>>>> write barriers.
>>>>>>>
>>>>>>> My solution to the problem is to introduce a platform-specific GC
>>>>>>> barrier set code generator. The abstract super class is
>>>>>>> BarrierSetCodeGen, and you can get it from the active BarrierSet. A
>>>>>>> virtual call to the BarrierSetCodeGen generates the relevant GC 
>>>>>>> barriers
>>>>>>> for the arraycopy stub routines.
>>>>>>>
>>>>>>> The BarrierSetCodeGen inheritance hierarchy exactly matches the
>>>>>>> corresponding BarrierSet inheritance hierarchy. In other words, 
>>>>>>> every
>>>>>>> BarrierSet class has a corresponding BarrierSetCodeGen class.
>>>>>>>
>>>>>>> The various switch statements that generate different GC barriers
>>>>>>> depending on the enum type of the barrier set have been changed 
>>>>>>> to call
>>>>>>> a corresponding virtual member function in the BarrierSetCodeGen 
>>>>>>> class
>>>>>>> instead.
>>>>>>>
>>>>>>> Thanks to Martin Doerr and Roman Kennke for providing platform 
>>>>>>> specific
>>>>>>> code for PPC, S390 and AArch64.
>>>>>>>
>>>>>>> Webrev:
>>>>>>> http://cr.openjdk.java.net/~eosterlund/8198949/webrev.00/
>>>>>>>
>>>>>>> CR:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8198949
>>>>>>>
>>>>>>> Thanks,
>>>>>>> /Erik
>>>>>>
>>>>>> I looked over x86, aarch64 and shared code (in webrev.01), and it 
>>>>>> looks
>>>>>> good to me!
>>>>>>
>>>>>> As I commented earlier in private, I would find it useful if the
>>>>>> barriers could 'take over' the whole arraycopy, for example to do 
>>>>>> the
>>>>>> pre- and post-barrier and arraycopy in one pass, instead of 3. 
>>>>>> However,
>>>>>> let's keep that for later.
>>>>>>
>>>>>> Awesome work, thank you!
>>>>>>
>>>>>> Cheers,
>>>>>> Roman
>>>>>>
>>>>>>
>>>>>
>>>>
>>