RFR: JDK-8207169: X86: Modularize cmpxchg-oop assembler for C1 and C2

[Roman Kennke]

Hmm right, generating specific e.g. ShenandoahCompareAndSwapPNode and
putting an expansion for that in the .ad file seems like a good idea.

We tried generating ideal graph for what we need for cmpxchg-oop and it
was complicated and error-prone and not really worth it. We may try again.

Roman

> I agree with you that it would be nice to have a good interface for
> this, and that putting your GC barriers straight into the AD file makes
> it very awkward to have a clean interface. It will be a special
> interface for shenandoah only, that only shenandoah uses. All other GCs
> generate nodes for their barriers. It seems like a bit of a shortcut for
> not having to generate the proper nodes for your barriers, which I think
> is the right level to do this, and is where all other GCs do this. The
> rules matched in the AD file kind of presume you know what instructions
> will be generated. This is required for example for PrintOptoAssembly to
> generate anything useful. It might not be a dealbreaker I suppose if
> PrintOptoAssembly prints nonsense. But I can't help but feel like the
> abstraction level is not right here, and that placing GC barriers
> straight into the AD file for convenience, is a bit of a hack.
> 
> An alternative that I think should be considered, is to do what ZGC
> does: generate nodes in ZBarrierSetC2, and macro expand barrier nodes in
> ZBarrierSetC2. That way, your changes can remain in your
> ShenandoahBarrierSetC2 class, and not leak out into the AD files.
> 
> Thanks,
> /Erik
> 
> On 2018-08-29 16:35, Roman Kennke wrote:
>> Hi Roland,
>>
>>>> It was suggested to me (off-list) to simply put this stuff under if
>>>> (UseShenandoahGC). I don't have a very strong preference about this.
>>>> I'm
>>>> leaning towards having a (somewhat) proper GC interface for it, if only
>>>> because of the symmetry with the runtime GC interface that also has a
>>>> cmpxchg hook, and because, cmpxchg(-oop) is, in-fact, a heap-access and
>>>> should thus go through the GC interface.
>>>>
>>>> So, how do others feel about this? Better to put this under
>>>> Shenandoah-specific paths? Make it a proper GC/BarrierSet-interface? I
>>>> also very much welcome suggestions for improving the suggested
>>>> interface.
>>> FWIW, I made that off-list comment. My objection is that:
>>>
>>>    666     if
>>> (BarrierSet::barrier_set()->barrier_set_assembler()->handle_cmpxchg_oop())
>>> {
>>>    667       tmp1 = new_register(T_OBJECT);
>>>    668       tmp2 = new_register(T_OBJECT);
>>>    669     }
>>>
>>> is shenandoah specific. I suppose you could have an API entry point that
>>> would return some opaque object. That object for shenandoah would record
>>> 2 extra registers. Then in the backend, you would pass that opaque
>>> object to BarrierSetAssembler::cmpxchg_oop_c1() and the shenandoah
>>> implementation would get its 2 extra registers.
>>>
>>> The problem is that with c2:
>>>
>>>   7471 instruct compareAndSwapP_BS(rRegI res,
>>>   7472                             memory mem_ptr,
>>>   7473                             rRegP tmp1, rRegP tmp2,
>>>   7474                             rax_RegP oldval, rRegP newval,
>>>   7475                             rFlagsReg cr)
>>>   7476 %{
>>>
>>> is also shenandoah specific for the same reason (it needs 2 extra
>>> registers). And I don't see a way to properly abstract that.
>>>
>>> So it seems to me, that code is not properly abstracted, there's no
>>> reasonable way to properly abstract it, and it's actually shenandoah
>>> specific code in disguise so it's better to have it be explicitly
>>> shenandoah specific.
>>>
>>> Roland.
>>>
>>
>> Thanks, Roland!
>>
>> So let me put my argument here too. The GC/BarrierSet-interface already
>> has a couple of interfaces that are Shenandoah-specific. Basically all
>> of what I added for supporting primitive access, object equality and
>> allocations is Shenandoah-specific. There is a cmpxchg-oop interface in
>> the runtime too. But then again, it is totally conceivable that a future
>> GC might need those abstractions. In-fact, as far as I know, ZGC suffers
>> from a similar problem with cmpxchg-oop as Shenandoah does, but there
>> it's solved differently.
>>
>> Looking from a very-far-away perspective, one of the design ideas behind
>> the current GC/BarrierSet-interface was that the GC should own all heap
>> access. And cmpxchg-oop is quite definitely a heap-access. It is
>> unfortunate that we can't seem to make it really clean with the way that
>> the C2 backend is currently working. What I suggested seemed the closest
>> we can get while remaining practical.
>>
>> Roman
>>
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20180829/09aa0ea7/signature.asc>