[aarch64-port-dev ] Question about ISB usage in the aarch64 port

[Andrew Haley]

On 11/21/19 11:47 AM, Yangfei (Felix) wrote:
>> -----Original Message-----
>> From: Andrew Haley [mailto:aph at redhat.com]
>> Sent: Thursday, November 21, 2019 6:04 PM
>> To: Yangfei (Felix) <felix.yang at huawei.com>;
>> aarch64-port-dev at openjdk.java.net
>> Subject: Re: [aarch64-port-dev ] Question about ISB usage in the aarch64 port
>>
>> On 11/15/19 8:33 AM, Yangfei (Felix) wrote:
>>>
>>>   The rt_call function is used in generate_native_wrapper to make the
>>>   JNI call.
>>>   As I didn’t see the barrier for the ppc or arm port.  I would like
>>>   to know more details here.  Does anyone still remember?
>>
>> What question are you asking? The ISB is there because the callout might run
>> concurrently with a safepoint, during which time the code cache may be
>> changed by some other thread. While we are in native code safepoints can run
>> in other threads without us knowing.
> 
> I didn't find this barrier for the ppc or arm port.  
> My question: is this necessary to plant a instruction barrier in the same place for those ports?  

Probably. There was recently some code discussed to do pipeline
flushing for for x86 and others. I can't find it right now...

>>>   Also the ISB is planted only in the else block.  I assume this is
>>>   also necessary for the if block.  Correct?
>>
>> No. The if block is for calls within the AArch64 Java runtime, so
>> we stay in Java, and there shouldn't be any ISB needed. Any part of
>> the Java runtime that loads or generates code does its own ISB.
> 
> I see.  Does that mean the isb in LIR_Assembler::rt_call should be planted in the else block? 
> I mean: 
> 
> diff -r dc45ed0ab083 src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp
> --- a/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp       Wed Nov 13 15:16:45 2019 -0800
> +++ b/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp       Thu Nov 21 19:25:00 2019 +0800
> @@ -2906,12 +2906,12 @@
>    } else {
>      __ mov(rscratch1, RuntimeAddress(dest));
>      __ blr(rscratch1);
> +    __ maybe_isb();
>    }
> 
>    if (info != NULL) {
>      add_call_info_here(info);
>    }
> -  __ maybe_isb();
>  }

Quite possibly, but I wouldn't touch it without a very good reason and
lots of analysis.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671