[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR (12) JDK-8218025: disable pop_frame and force_early_return caps for Graal
From:       "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date:       2019-01-31 19:39:35
Message-ID: 79d984ba-f3bd-b329-4d5f-4ba2083e9628 () oracle ! com
[Download RAW message or body]

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@oracle.com wrote:
>> On 1/30/19 21:24, David Holmes wrote:
>>> On 31/01/2019 3:18 pm, dean.long@oracle.com wrote:
>>>> On 1/30/19 8:59 PM, serguei.spitsyn@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
>>>>
>>

[prev in list] [next in list] [prev in thread] [next in thread]