RFR (12) JDK-8218025: disable pop_frame and force_early_return caps for Graal

[David Holmes]

+1

David

On 1/02/2019 5:39 am, serguei.spitsyn at oracle.com wrote:
> Hi Alex,
> 
> Looks fine to me.
> 
> Thanks,
> Serguei
> 
> 
> On 1/31/19 10:38, Alex Menkov wrote:
>> Hi guys,
>>
>> thank you for the feedback.
>>
>> updated webrev (used the way suggested by David & Igor):
>> http://cr.openjdk.java.net/~amenkov/tck_red_disable_caps/webrev.02/
>>
>> --alex
>>
>> On 01/30/2019 21:31, serguei.spitsyn at oracle.com wrote:
>>> On 1/30/19 21:24, David Holmes wrote:
>>>> On 31/01/2019 3:18 pm, dean.long at oracle.com wrote:
>>>>> On 1/30/19 8:59 PM, serguei.spitsyn at oracle.com wrote:
>>>>>> So, the fix needs to be more like this:
>>>>>> + // Workaround for 8195635:
>>>>>> + // disable pop_frame and force_early_return capabilities with Graal
>>>>>> + #if INCLUDE_JVMCI
>>>>>> + if (!(EnableJVMCI && UseJVMCICompiler)) {
>>>>>>     jc.can_pop_frame = 1;
>>>>>>     jc.can_force_early_return = 1;
>>>>>> + } + #endif Not sure, if the check for EnableJVMCI can be removed 
>>>>>> above.
>>>>>
>>>>> We still need it to work when INCLUDE_JVMCI is not defined.
>>>>> How about
>>>>>
>>>>> JVMCI_ONLY(if (UseJVMCICompiler)) {
>>>>> ...
>>>>> }
>>>>>
>>>>> or
>>>>>
>>>>> if (JVMCI_ONLY(UseJVMCICompiler) NOT_JVMCI(true)) {
>>>>> ...
>>>>> }
>>>>
>>>> Or just turn them on unconditionally first and turn off explicitly 
>>>> for JVMCI:
>>>>
>>>>  jc.can_pop_frame = 1;
>>>>  jc.can_force_early_return = 1;
>>>> + #if INCLUDE_JVMCI
>>>> +  // Workaround for 8195635:
>>>> +  // disable pop_frame and force_early_return capabilities with Graal
>>>> + if (EnableJVMCI && UseJVMCICompiler) {
>>>> +     jc.can_pop_frame = 0;
>>>> +     jc.can_force_early_return = 0;
>>>> + }
>>>> + #endif
>>>>
>>> Oh, Dean is right.
>>> We need these caps initialized even if the macro INCLUDE_JVMCI is 
>>> undefined.
>>> Then I like variant from David above.
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>>> David
>>>>
>>>>> dl
>>>>>
>>>
>