RFR: 8201543: Modularize C1 GC barriers

[Vladimir Kozlov]

Hi Erik,

I would suggest for Rickard and Nils become reviewers of these changes 
since they more than anyone in our group know about them. The should do 
review on this thread.

And CCing to Graal group because Graal may need to be changes too.

Thanks,
Vladimir

On 4/20/18 6:49 AM, Erik Österlund wrote:
> Hi everyone,
> 
> I sat down with Rickard and Per at the office to have a look at this, 
> and have built a new webrev based on their feedback.
> The main elements in the delta are the following:
> 
> 1) Wrap various context information that is passed around in the 
> BarrierSetC1 hierarchy in a wrapper object (to reduce boiler plate), 
> that has been named LIRAccess. It contains the address elements (base 
> and offset, as either LIRItem or LIROpr), as well as the decorators, and 
> the CodeEmitInfo of the address (for patching), the CodeEmitInfo for the 
> access (for things like implicit null checks), and the LIRGenerator 
> instance, that would normally be passed around to every function.
> 2) Added #ifdef COMPILER1 in the G1BarrierSetC1 classes to be polite to 
> people trying to build HotSpot with a generated interpreter but no C1 
> compiler.
> 3) Added a decorator_fixup() method that applies various implicit 
> decorator rules for sane defaults (for example, IN_HEAP_ARRAY implies 
> IN_HEAP). This is a 1:1 mirror to the DecoratorFixup meta function used 
> in the runtime backend. Both are now located in accessDecorator.hpp. One 
> for use by templates (DecoratorFixup), and one for use by code 
> generators that do not use templates (decorator_fixup()).
> 4) Removed some unnecessary includes and friend class declarations.
> 5) Made BarrierSetC1 a member of LIRGenerator to reduce boiler plate 
> further.
> 6) Changed name of the lir_generator variable passed around to gen, to 
> be consistent with what other code in C1 does when passing around the 
> LIRGenerator instance.
> 
> Incremental webrev:
> http://cr.openjdk.java.net/~eosterlund/8201543/webrev.00_01/
> 
> Full webrev:
> http://cr.openjdk.java.net/~eosterlund/8201543/webrev.01/
> 
> Big thanks to Rickard and Per for having a look at this.
> 
> Thanks,
> /Erik
> 
> On 2018-04-13 17:11, Erik Österlund wrote:
>> Hi,
>>
>> The GC barriers for C1 are not as modular as they could be. It 
>> currently uses switch statements to check which GC barrier set is 
>> being used, and calls one or another barrier based on that, in a way 
>> that it can only be used for write barriers.
>>
>> The solution I propose is to add the same facilities that have been 
>> added in runtime and the interpreter already: a barrier set backend 
>> for C1. I call it BarrierSetC1, and it helps us generate decorated 
>> accesses that give the GC control over the details how to generate 
>> this access. It recognizes the same decorators (accessDecorators.hpp) 
>> that the other parts of the VM recognize. Each concrete barrier set 
>> has its own backend. For now, these are CardTableBarrierSetC1 and 
>> G1BarrierSetC1, but this should pave way for upcoming concurrently 
>> compacting GCs as well.
>>
>> Two decorators were added for C1 specifically (found in 
>> c1_Decorators.hpp):
>> C1_NEEDS_PATCHING for accesses where the index is not yet load because 
>> the class has yet to be loaded, and
>> C1_MASK_BOOLEAN for accesses that need to mask untrusted boolean values.
>>
>> LIRGenerator calls a wrapper called access_store_at, access_load_at, 
>> etc (there are variants for cpmxchg, xchg and atomic add as well).
>> The access call calls straight into the BarrierSetC1 hierarchy using 
>> virtual calls. It is structured in a way very similar to 
>> BarrierSetAssembler.
>>
>> BarrierSetC1 can also be called during initialization to generate 
>> stubs and runtime methods required by C1. For G1BarrierSetC1, this 
>> results in calling the BarrierSetAssembler for the platform specific 
>> code. This way, the BarrierSetC1 hierarchy has been carefully kept in 
>> shared code, and the switch statements for generating G1 code have 
>> been removed. Some code that used to be platform specific (like unsafe 
>> get/set and array store) have been broken out to shared code, with the 
>> actual platform specific details (some register allocation for store 
>> check and atomics) broken out to platform specific methods. This way, 
>> calls to access are kept in platform specific code.
>>
>> As usual, big thanks go to Martin Doerr for helping out with S390 and 
>> PPC, and Roman for taking care of AArch64.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8201543
>>
>> Webrev:
>> http://cr.openjdk.java.net/~eosterlund/8201543/webrev.00/
>>
>> Thanks,
>> /Erik
>