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

[Vladimir Kozlov]

Sorry, my bad - C2 supports long shifts in 32-bit JVM unconditionally. 
Your previous hotspot.04/webrev/ looks fine then regarding C2.

But :) C1 restriction could be improved. I looked on jdk 8 code 
(wondering it works) and it does not have shift.

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

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