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

[Robbin Ehn]

Thanks!

/Robbin

On 08/30/2016 07:06 PM, Vladimir Kozlov wrote:
> Looks good.
>
> Thanks,
> Vladimir
>
> On 8/30/16 2:08 AM, Robbin Ehn wrote:
>> 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_LIRAssembler_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