[9] RFR (S): 8172844: Assert fails in deoptimization due to original PC at the end of code section

[Zoltán Majó]

Hi Rickard,


On 01/20/2017 10:49 AM, Rickard Bäckman wrote:
> [...]
>
>>> Another solution would be to always insert at least 1 hlt
>>> instruction after a call that isn't supposed to return.
>>> Or at least always if the call is the last instruction in the last block to be
>>> emitted.
>> Yes, that would work. Do you think we should file an enhancement for that?
> I think it is a good idea since it will avoid any situation where there
> is confusion about whether it is right to use stub_begin() >= or
> stub_begin() > as well as inst_end() <= and inst_end() <

I agree. I filed the following enhancement to address the problem:

8173184: Emit padding instructions after non-returning calls
https://bugs.openjdk.java.net/browse/JDK-8173184

I assigned the enhancement to myself, but if you are interested in 
looking into it, please feel free to take it.

For the record: I'll push the fix today.

Thank you!

Best regards,


Zoltan

>
> /R
>
>> Best regards,
>>
>>
>> Zoltan
>>
>>> /R
>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 1/19/17 5:36 AM, Zoltán Majó wrote:
>>>>> Hi Rickard,
>>>>>
>>>>>
>>>>> On 01/19/2017 02:22 PM, Rickard Bäckman wrote:
>>>>>> Looks good to me.
>>>>> thank you for the review and for the suggestions.
>>>>>
>>>>>> Some clean up if you don't mind.
>>>>>>
>>>>>> frame_aarch64.inline.hpp:
>>>>>>
>>>>>> 85     assert(((CompiledMethod*)_cb)->insts_contains(_pc), "original
>>>>>> PC must be in CompiledMethod");
>>>>>> should use _cb->as_compiled_method() instead of the cast.
>>>>>>
>>>>>> frame_x86.inline.hpp:
>>>>>> same as above.
>>>>> That makes sense. Here is the updated webrev:
>>>>> http://cr.openjdk.java.net/~zmajo/8172844/webrev.01/
>>>>>
>>>>> Thank you!
>>>>>
>>>>> Best regards,
>>>>>
>>>>>
>>>>> Zoltan
>>>>>
>>>>>> Thanks
>>>>>> /R
>>>>>>
>>>>>> On 01/19, Zoltán Majó wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>>
>>>>>>> please review the fix for 8172844.
>>>>>>>
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8172844
>>>>>>> http://cr.openjdk.java.net/~zmajo/8172844/webrev.00/
>>>>>>>
>>>>>>> The assert checking the "original pc" (i.e., the pc that was on the
>>>>>>> stack before the stack location was overwritten to point to the
>>>>>>> deoptimization handler) is too strict. If the last instruction of
>>>>>>> the (to-be-)deoptimized method is a call, the pc will point to the
>>>>>>> instruction following the call. That instruction is outside of the
>>>>>>> code section (as the call is the last instruction in the code
>>>>>>> section).
>>>>>>>
>>>>>>> I suggest we relax the assert to accept the original pc being at the
>>>>>>> end of the code section as well. I tested the change with JPRT and
>>>>>>> locally with the reproducer, the failure did not appear. RBT is in
>>>>>>> progress.
>>>>>>>
>>>>>>> Thank you!
>>>>>>>
>>>>>>> Best regards,
>>>>>>>
>>>>>>>
>>>>>>> Zoltan
>>>>>>>