RFR(S): 8202399:) [C1] LIRGenerator::do_CheckCast needs to exclude is_invokespecial_receiver_check() when using PatchAlot

[David Holmes]

Thanks Vladimir! But as you actually supplied the patch details 
technically you can't self-review it. :)

Can I get a "second" review please.

Thanks,
David

On 1/05/2018 7:12 AM, Vladimir Ivanov wrote:
> Looks good.
> 
> Thanks a lot for taking care of it!
> 
> Best regards,
> Vladimir Ivanov
> 
> On 4/29/18 20:03, David Holmes wrote:
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8202399
>> webrev: http://cr.openjdk.java.net/~dholmes/8202399/webrev/
>>
>> JDK-8168699 introduced  a special case for 
>> is_invokespecial_receiver_check() in LIRGenerator::do_CheckCast:
>>
>>    } else if (x->is_invokespecial_receiver_check()) {
>>      assert(patching_info == NULL, "can't patch this");
>>      stub = new DeoptimizeStub(info_for_exception,
>>                                Deoptimization::Reason_class_check,
>>                                Deoptimization::Action_none);
>>
>> but overlooked a potential bad interaction with PatchAlot. This can 
>> lead to  the assertion failure as observed during testing for 
>> JDK-8200167 with Graal. The proposed fix (supplied by Vladimir) is 
>> quite simple but needs to be applied to each of the CPU specific files:
>>
>> diff --git a/src/hotspot/cpu/x86/c1_LIRGenerator_x86.cpp 
>> b/src/hotspot/cpu/x86/c1_LIRGenerator_x86.cpp
>> --- a/src/hotspot/cpu/x86/c1_LIRGenerator_x86.cpp
>> +++ b/src/hotspot/cpu/x86/c1_LIRGenerator_x86.cpp
>> @@ -1314,7 +1314,7 @@
>>      LIRItem obj(x->obj(), this);
>>
>>      CodeEmitInfo* patching_info = NULL;
>> -  if (!x->klass()->is_loaded() || (PatchALot && 
>> !x->is_incompatible_class_change_check())) {
>> +  if (!x->klass()->is_loaded() || (PatchALot && 
>> !x->is_incompatible_class_change_check() && 
>> !x->is_invokespecial_receiver_check())) {
>>
>> Testing:
>>   - hotspot/jtreg/compiler (complete)
>>   - hs-comp-precheckin (once mach5 is available)
>>
>> Thanks,
>> David