RFR (S): 8022494: Make compilation IDs sequential

[Albert Noll]

Hi Vladimir,

thanks for your explanation. I agree with your suggestion. The new 
version has the
corresponding check inside

*CompileBroker::assign_compile_id()*

 >Also why you return only in such case and not for normal native wrappers?

Thanks for catching this bug. I fixed it in the new version.


Here is the link to the new webrev:
http://cr.openjdk.java.net/~anoll/8022494/webrev.05/

Best,
Albert


On 12/21/2013 08:46 PM, Vladimir Kozlov wrote:
> We have to generate method_handle_intrinsic so we can't simple show 
> warning and continue execution - we can't do that.
> I suggested to generate compile_id always in such case and convert 
> your warning to assert (since it could only happens in debug VM).
> Also why you return only in such case and not for normal native wrappers?
>
> +       if (method->is_method_handle_intrinsic()) {
> +         warning("Must generate wrapper for method handle intrinsic");
> +         return;
> +       }
> ---
> +       assert(!method->is_method_handle_intrinsic()), "Must generate 
> wrapper for method handle intrinsic");
> +       return;
>
> Thanks,
> Vladimir
>
> On 12/18/13 10:18 PM, Albert Noll wrote:
>> Christian, Vladimir, thanks for the review.
>>
>> @Christian: Thanks for catching the typo
>>
>> @Vladimir: I am not sure if I understand your suggestion correctly. 
>> Could you please clarify what you
>> mean by "The warning above will be assert after that."
>>
>> Best,
>> Albert
>>
>>
>> On 10/28/2013 07:59 PM, Vladimir Kozlov wrote:
>>> Albert,
>>>
>>> The warning is not correct solution since we HAVE to generate method 
>>> handle intrinsics if your comment is correct:
>>>
>>> +       // must be generated for method handle intrinsics (8026407), 
>>> print out a warning.
>>> +       if (method->is_method_handle_intrinsic()) {
>>> +         warning("Must generate wrapper for method handle intrinsic");
>>> +         return;
>>> +       }
>>>
>>> I think assign_compile_id() should generate id in such case 
>>> regardless CIStart and CIStop values. The warning above
>>> will be assert after that.
>>>
>>> And, please, file RFE (starter task) to cleanup type of compile_id. 
>>> In some places it declared as 'int' and in an
>>> other as 'uint'.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 10/24/13 1:56 AM, Albert Noll wrote:
>>>> Here is the updated webrev:
>>>>
>>>> http://cr.openjdk.java.net/~anoll/8022494/webrev.04/ 
>>>> <http://cr.openjdk.java.net/%7Eanoll/8022494/webrev.04/>
>>>>
>>>> Best,
>>>> Albert
>>>>
>>>> On 24.10.2013 10:21, Albert Noll wrote:
>>>>> Hi Aleksey,
>>>>>
>>>>> thanks for looking at this.
>>>>>
>>>>> On 24.10.2013 10:15, Aleksey Shipilev wrote:
>>>>>> On 10/24/2013 12:01 PM, Albert Noll wrote:
>>>>>>> Here is the updated webrev:
>>>>>>> http://cr.openjdk.java.net/~anoll/8022494/webrev.03/
>>>>>> Nice to see the locking gone.
>>>>>>
>>>>>> compileBroker.cpp:
>>>>>>    * Is that considered correct that OSR and normal compilations are
>>>>>> marked differently when running in debug mode, but not in release? I
>>>>>> understand the comment before assign_compile_id, so this is more 
>>>>>> of the
>>>>>> philosophical question.
>>>>> Compilation IDs are only different if -XX:CICountOSR is set, which is
>>>>> defaulted to false.
>>>>>> sharedRuntime.cpp:
>>>>>>    * Why do you need "2653   return;" in the method tail?
>>>>> Thanks for spotting this. I missed it during the cleanup.
>>>>>
>>>>> Best,
>>>>> Albert
>>>>>> Thanks,
>>>>>> -Aleksey.
>>>>>
>>>>
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20140106/06ea5021/attachment-0001.html