PING Re: RFR: 8163589: Add back class id intrinsic method for event based tracing

[Robbin Ehn]

Thank you Markus!

/Robbin

On 08/30/2016 11:37 AM, Markus Gronlund wrote:
> Hi Robbin,
>
> Looks good (still).
>
> Thanks
> Markus
>
> -----Original Message-----
> From: Robbin Ehn
> Sent: den 30 augusti 2016 11:09
> To: Vladimir Kozlov; hotspot-compiler-dev at openjdk.java.net; Markus Grönlund
> Subject: Re: PING Re: RFR: 8163589: Add back class id intrinsic method for event based tracing
>
> Hi,
>
> On 08/29/2016 07:22 PM, Vladimir Kozlov wrote:
>> Sorry, my bad - C2 supports long shifts in 32-bit JVM unconditionally. Your previous hotspot.04/webrev/ looks fine then regarding C2.
>
> Ok, thanks.
>
>>
>> But :) C1 restriction could be improved. I looked on jdk 8 code (wondering it works) and it does not have shift.
>
> Yes, shift is new in jdk9.
>
>>
>> Assuming that shift is not always defined the code in c1_Compiler.cpp could be:
>>
>> +#if defined(_LP64) || !defined(TRACE_ID_CLASS_SHIFT)
>> +  case vmIntrinsics::_getClassId:
>> +#endif
>
> Yes, incremental on top of version 04:
> Inc: http://cr.openjdk.java.net/~rehn/8163589/hotspot.04-06/
> Full: http://cr.openjdk.java.net/~rehn/8163589/hotspot.06/
>
> Tested with jmh x64/x86 with PrintIntrinsics/PrintInlining.
>
> Markus you looked at v04, I hope you are also are fine with this?
>
> Thanks!
>
> /Robbin
>
>>
>> Thanks,
>> Vladimir
>>
>> On 8/29/16 5:10 AM, Robbin Ehn wrote:
>>> Thanks for looking at this again,
>>>
>>> On 08/26/2016 08:28 PM, Vladimir Kozlov wrote:
>>>> C2 code also should be guarded by #ifdef _LP64
>>>
>>> Here is the webrev for that:
>>> Inc:http://cr.openjdk.java.net/~rehn/8163589/hotspot.04-05/webrev/
>>> Full:http://cr.openjdk.java.net/~rehn/8163589/hotspot.05/webrev/
>>>
>>> Tested with jmh, jdk_jfr and new intrinsic test(closed).
>>>
>>> It's not obvious to me why, since tests work on at least x86.
>>> Would you mind sharing why this is needed?
>>>
>>> Thanks!
>>>
>>> /Robbin
>>>
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 8/26/16 12:36 AM, Robbin Ehn wrote:
>>>>> Hi all,
>>>>>
>>>>> Please have a look, thanks!
>>>>>
>>>>> /Robbin
>>>>>
>>>>> (updated issue heading also)
>>>>>
>>>>> On 08/23/2016 06:06 PM, Robbin Ehn wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> C1 in 32-bit actually don't support shift operation which was
>>>>>> added in
>>>>>> v3 of this series.
>>>>>>
>>>>>> We hit this assert in 32-bit:
>>>>>> #  Internal Error
>>>>>> (/home/rehn/source/jdk/8163589/hotspot/src/cpu/x86/vm/c1_LIRAssemb
>>>>>> ler_x86.cpp:2917),
>>>>>>
>>>>>>
>>>>>> pid=22445, tid=22493
>>>>>> #  Error: Unimplemented()
>>>>>>
>>>>>> So we will just live without this intrinsic in C1 32-bit.
>>>>>>
>>>>>> Also added specific test for intrinsic and 'event based testing',
>>>>>> see bug (https://bugs.openjdk.java.net/browse/JDK-8163589).
>>>>>>
>>>>>> I really hope I haven't missed anything else!
>>>>>>
>>>>>> Incr:
>>>>>> http://cr.openjdk.java.net/~rehn/8163589/hotspot.03-04/webrev/
>>>>>>
>>>>>> Full:
>>>>>> http://cr.openjdk.java.net/~rehn/8163589/hotspot.04/webrev/
>>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>> /Robbin
>>>>>>
>>>>>> On 08/10/2016 04:05 PM, Robbin Ehn wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> In 8 we had this intrinsic, I'm adding it back Used for event
>>>>>>> based tracing.
>>>>>>>
>>>>>>> Webrev:
>>>>>>> http://cr.openjdk.java.net/~rehn/8163589/hotspot.01/webrev/
>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8163589
>>>>>>>
>>>>>>> Thanks!
>>>>>>>
>>>>>>> /Robbin