Testing hotspot's Atomic::cmpxchg(jbyte*) ?

[Aleksey Shipilev]

On 08/19/2016 03:03 PM, David Holmes wrote:
> On 19/08/2016 7:37 PM, Aleksey Shipilev wrote:
>> On 08/19/2016 12:26 PM, David Holmes wrote:
>>> On 19/08/2016 5:40 PM, Aleksey Shipilev wrote:
>>>> On 08/19/2016 10:13 AM, David Holmes wrote:
>>>>>> Now it might be cleaner to ditch Java version from Unsafe, and make
>>>>>> native entry points like Unsafe_CompareAnd{Exchange,Swap}{Byte,Short}
>>>>>> which would call relevant Atomic::cmpxchg-s.
>>>>>
>>>>> I tried commenting out the Java-ish version and defining one that
>>>>> would
>>>>> call Atomic::cmpxchg from unsafe.cpp in the VM. However something is
>>>>> complaining about the intrinisics - I removed the
>>>>> HotspotIntrinsicCandidate annotation as I don't want any intrinisics,
>>>>> but I get a build error:
>>>>>
>>>>> Compiler intrinsic is defined for method
>>>>> [jdk.internal.misc.Unsafe.compareAndSwapByte(Ljava/lang/Object;JBB)Z],
>>>>> but the method is not annotated with @HotSpotIntrinsicCandidate.
>>>>> Exiting.
>>>>>
>>>>> No idea where this is coming from or how I can disable it ??
>>>>
>>>> @HotSpotIntrinsicCandidate marks Java methods that are declared in
>>>> vmSymbols.hpp, and the VM code checks it. So, removing @HSIC without
>>>> modifying vmSymbols.hpp would blow up like that.
>>>>
>>>> Anyway, I think you have to leave @HSIC methods untouched, because C2
>>>> would intrinsify them directly. To cater for the unsafe.cpp slow path,
>>>> do a separate method:
>>>>
>>>>     @HotSpotIntrinsicCandidate
>>>>     public final byte compareAndExchangeByteVolatile(Object o, long
>>>> offset,
>>>>                                                      byte expected,
>>>>                                                      byte x) {
>>>>         compareAndExchangeByte0(o, long, expected, x);
>>>>     }
>>>>
>>>>     public final native byte compareAndExchangeByte0(...);
>>>>
>>>> ...and then do an entry point for it in unsafe.cpp.
>>>
>>> Okay that was the easy part.
>>>
>>> Now where do I find out the conditional compilation syntax for the
>>> X-VarHandle.java.template file?
>>
>> The template itself is driven by Spp preprocessor, the same one used
>> thorough JDK. It is called during build via
>> ./jdk/make/gensrc/GensrcVarHandles.gmk.
> 
> Yes but I don't know the syntax.

If you find Spp.java in the forest, there is a comment section about the
syntax:
   ./jdk/make/src/classes/build/tools/spp/Spp.java


>> But why do you need to change VH templates? VH implementations call
>> Unsafe, see:
>>
>>         @ForceInline
>>         static $type$ compareAndExchange(FieldInstanceReadWrite handle,
>> Object holder, $type$ expected, $type$ value) {
>>             return
>> UNSAFE.compareAndExchange$Type$Volatile(Objects.requireNonNull(handle.receiverType.cast(holder)),
>>
>>                                                handle.fieldOffset,
>>
>> {#if[Object]?handle.fieldType.cast(expected):expected},
>>
>> {#if[Object]?handle.fieldType.cast(value):value});
>>         }
>>
>> ...so once you do the Unsafe part above, everything should be linked
>> together. The jtreg tests exercise VH byte ops, so you can run them.
> 
> I was only making a change to the Byte version so the template needs to
> be specialized for Byte to call CompareAndExchangeByte0.

I get that.

But I still think X-VarHandle.template change is unnecessary. The code
for byte CAS generated from X-VarHandle.template calls
Unsafe.compareAnd{Swap,Exchange}*. Why can't you leave VH template
alone, and call CompareAndExchangeByte0 from
Unsafe.compareAndExchangeByteVolatile, like I suggested before?

     @HotSpotIntrinsicCandidate
     public final byte compareAndExchangeByteVolatile(Object o,
                                                      long  offset,
                                                      byte expected,
                                                     byte x) {
         compareAndExchangeByte0(o, long, expected, x);
     }

     public final native byte compareAndExchangeByte0(...);

VH tests have a least one testing mode which does not intrinsify Unsafe
methods, i.e. -Xint. compareAndExchangeByte0 path would be visited there.

-Aleksey