RFR: 8198949: Modularize arraycopy stub routine GC barriers

[Per Liden]

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