RFR(m): 8220351: Cross-modifying code

[Doerr, Martin]

Hi Robbin,

for PPC, isync came into my mind first (also mentioned by David). However, isync is only strictly required for a thread which performs modifications and wants to execute these modified instructions. We already have isync at the patch sites.

The normal "fence" = "sync" does the job for cross-modification on PPC, so I don't think we need to execute any additional instructions.
The existing barriers look fine for PPC.

I believe other non-x86 platforms are also fine with existing fences (s390, haven't checked others).

For x86, why don't we just "upgrade" the few existing barriers to include instruction stream synchronization?
E.g.
sharedRuntime_x86_64.cpp: 2564
templateInterpreterGenerator_x86.cpp: 1094
safepoint.cpp: 822
...

Am I missing anything?

Best regards,
Martin


-----Original Message-----
From: Robbin Ehn <robbin.ehn at oracle.com> 
Sent: Dienstag, 12. März 2019 10:44
To: Doerr, Martin <martin.doerr at sap.com>; Andrew Haley <aph at redhat.com>; hotspot-dev at openjdk.java.net
Subject: Re: RFR(m): 8220351: Cross-modifying code

Hi Martin,

On 3/11/19 6:43 PM, Doerr, Martin wrote:
> Hi Robbin,
> 
>> They are only changed during a GC safepoint on archs with
>> mustIterateImmediateOopsInCode set to true, AFAIK.
> 
> Only x86 uses Oops directly encoded into the instruction stream.
> But all platforms have compressed Oops encoded: loadConN nodes in ad files.

Sorry, forgot about that. Was just looking at the immediate ones.

> 
> I wonder if there are already barriers in place which have the required effect for non-x86 CPUs.
> For example fence() and acquire() already include instruction stream synchronization on PPC.
> Is that why you implemented the new barrier empty for non-x86 CPUs?

Partly because wasn't sure e.g. isync was what you wanted.
And maybe someone would choose to gamble to not take the performance hit.
Or there may be other considering for that arch which I'm not aware of.
(I wasn't aware of arms need to sync I-cache with D-cache, I thought ISB was enough)

Thanks, Robbin

> 
> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of Robbin Ehn
> Sent: Montag, 11. März 2019 13:56
> To: Andrew Haley <aph at redhat.com>; hotspot-dev at openjdk.java.net
> Subject: Re: RFR(m): 8220351: Cross-modifying code
> 
> On 2019-03-11 11:26, Andrew Haley wrote:
>> On 3/8/19 3:24 PM, Robbin Ehn wrote:
>>
>>> Issue:
>>> https://bugs.openjdk.java.net/browse/JDK-8220351
>>> Changeset:
>>> http://cr.openjdk.java.net/~rehn/8220351/webrev/
>>>
>>> After a JavaThread have been in a safepoint/(handshake) safe state
>>> it can start executing updated code. E.g. an oop in the instruction
>>> stream can have been updated.
>>
>> Hmm, interesting. I think it works on on AArch64 at present because
>> the segfault trap we take is effectively a full synchronization
>> operation.
>>
>> An AArch64 ISB (instruction synchronization barrier) invalidates the
>> pipeline. It doesn't invalidate the instruction cache, which we'd need
>> to do to see a relocated OOP. If an instruction is modified by some
>> other thread we need to flush the local icache, but to do that we need
>> to know which instructions have been changed. We could create a
>> modification queue, but that seems rather elaborate.
>>
>> What is is that changes OOPs in the instructions during a handshake?
>> Is it just garbage collection, or does it happen at other times too?
> 
> They are only changed during a GC safepoint on archs with
> mustIterateImmediateOopsInCode set to true, AFAIK.
> Arm and aarch64 have it false.
> 
> /Robbin
> 
>>
>> We could simply move OOPs (and class metadata pointers?) into the
>> constant pool. That would have some performance impact on in-order
>> CPUs, but hopefully not much on out-of-order ones.
>>
> 
>