RFR: 8174961: [JVMCI] incorrect implementation of isCompilable

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8174961: [JVMCI] incorrect implementation of isCompilable

Doug Simon @ Oracle
Please review this fix for a rather embarrassing bug I committed as part of JDK-8172733.

http://cr.openjdk.java.net/~dnsimon/8174961/webrev/
https://bugs.openjdk.java.net/browse/JDK-8174961

-Doug
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8174961: [JVMCI] incorrect implementation of isCompilable

Vladimir Kozlov
I did not get the condition. Why a method is *compilable* if JVMCICompiler is not used? Should it be

UseJVMCICompiler && !method->is_not_compilable(

Then comment is wrong. Please, explain.

Thanks,
Vladimir

On 2/14/17 12:11 PM, Doug Simon wrote:
> Please review this fix for a rather embarrassing bug I committed as part of JDK-8172733.
>
> http://cr.openjdk.java.net/~dnsimon/8174961/webrev/
> https://bugs.openjdk.java.net/browse/JDK-8174961
>
> -Doug
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8174961: [JVMCI] incorrect implementation of isCompilable

Doug Simon @ Oracle

> On 14 Feb 2017, at 21:28, Vladimir Kozlov <[hidden email]> wrote:
>
> I did not get the condition. Why a method is *compilable* if JVMCICompiler is not used? Should it be
>
> UseJVMCICompiler && !method->is_not_compilable(
>
> Then comment is wrong. Please, explain.

If UseJVMCICompiler is false, then JVMCI is not being used by the VM as a jit but is only being used in hosted mode (e.g. by Truffle). The thinking was that hosted clients do not typically care about VM policies effecting the compilability of a method (e.g. whether it has a breakpoint etc). However, now that I reflect on this further, such clients should ignore this call altogether as they may be in a VM environment where both hosted and non-hosted compilations are being requested of JVMCI. With that in mind, I’ve updated the webrev to remove all mention and use of UseJVMCICompiler.

-Doug

>
> On 2/14/17 12:11 PM, Doug Simon wrote:
>> Please review this fix for a rather embarrassing bug I committed as part of JDK-8172733.
>>
>> http://cr.openjdk.java.net/~dnsimon/8174961/webrev/
>> https://bugs.openjdk.java.net/browse/JDK-8174961
>>
>> -Doug
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8174961: [JVMCI] incorrect implementation of isCompilable

Vladimir Kozlov
Should you also remove flag check in previous lines in the test?:

          boolean isCompilable = CompilerToVMHelper.isCompilable(method);
          boolean expected = UseJVMCICompiler || WB.isMethodCompilable(aMethod);

Thanks,
Vladimir

On 2/14/17 12:40 PM, Doug Simon wrote:

>
>> On 14 Feb 2017, at 21:28, Vladimir Kozlov <[hidden email]> wrote:
>>
>> I did not get the condition. Why a method is *compilable* if JVMCICompiler is not used? Should it be
>>
>> UseJVMCICompiler && !method->is_not_compilable(
>>
>> Then comment is wrong. Please, explain.
>
> If UseJVMCICompiler is false, then JVMCI is not being used by the VM as a jit but is only being used in hosted mode (e.g. by Truffle). The thinking was that hosted clients do not typically care about VM policies effecting the compilability of a method (e.g. whether it has a breakpoint etc). However, now that I reflect on this further, such clients should ignore this call altogether as they may be in a VM environment where both hosted and non-hosted compilations are being requested of JVMCI. With that in mind, I’ve updated the webrev to remove all mention and use of UseJVMCICompiler.
>
> -Doug
>
>>
>> On 2/14/17 12:11 PM, Doug Simon wrote:
>>> Please review this fix for a rather embarrassing bug I committed as part of JDK-8172733.
>>>
>>> http://cr.openjdk.java.net/~dnsimon/8174961/webrev/
>>> https://bugs.openjdk.java.net/browse/JDK-8174961
>>>
>>> -Doug
>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8174961: [JVMCI] incorrect implementation of isCompilable

Doug Simon @ Oracle

> On 14 Feb 2017, at 21:47, Vladimir Kozlov <[hidden email]> wrote:
>
> Should you also remove flag check in previous lines in the test?:
>
>         boolean isCompilable = CompilerToVMHelper.isCompilable(method);
>         boolean expected = UseJVMCICompiler || WB.isMethodCompilable(aMethod);
>

Yes, you’re right - webrev is updated with this change. Thanks for catching this oversight!

-Doug

> Thanks,
> Vladimir
>
> On 2/14/17 12:40 PM, Doug Simon wrote:
>>
>>> On 14 Feb 2017, at 21:28, Vladimir Kozlov <[hidden email]> wrote:
>>>
>>> I did not get the condition. Why a method is *compilable* if JVMCICompiler is not used? Should it be
>>>
>>> UseJVMCICompiler && !method->is_not_compilable(
>>>
>>> Then comment is wrong. Please, explain.
>>
>> If UseJVMCICompiler is false, then JVMCI is not being used by the VM as a jit but is only being used in hosted mode (e.g. by Truffle). The thinking was that hosted clients do not typically care about VM policies effecting the compilability of a method (e.g. whether it has a breakpoint etc). However, now that I reflect on this further, such clients should ignore this call altogether as they may be in a VM environment where both hosted and non-hosted compilations are being requested of JVMCI. With that in mind, I’ve updated the webrev to remove all mention and use of UseJVMCICompiler.
>>
>> -Doug
>>
>>>
>>> On 2/14/17 12:11 PM, Doug Simon wrote:
>>>> Please review this fix for a rather embarrassing bug I committed as part of JDK-8172733.
>>>>
>>>> http://cr.openjdk.java.net/~dnsimon/8174961/webrev/
>>>> https://bugs.openjdk.java.net/browse/JDK-8174961
>>>>
>>>> -Doug
>>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8174961: [JVMCI] incorrect implementation of isCompilable

Vladimir Kozlov
Good.

Thanks,
Vladimir

On 2/14/17 12:59 PM, Doug Simon wrote:

>
>> On 14 Feb 2017, at 21:47, Vladimir Kozlov <[hidden email]> wrote:
>>
>> Should you also remove flag check in previous lines in the test?:
>>
>>         boolean isCompilable = CompilerToVMHelper.isCompilable(method);
>>         boolean expected = UseJVMCICompiler || WB.isMethodCompilable(aMethod);
>>
>
> Yes, you’re right - webrev is updated with this change. Thanks for catching this oversight!
>
> -Doug
>
>> Thanks,
>> Vladimir
>>
>> On 2/14/17 12:40 PM, Doug Simon wrote:
>>>
>>>> On 14 Feb 2017, at 21:28, Vladimir Kozlov <[hidden email]> wrote:
>>>>
>>>> I did not get the condition. Why a method is *compilable* if JVMCICompiler is not used? Should it be
>>>>
>>>> UseJVMCICompiler && !method->is_not_compilable(
>>>>
>>>> Then comment is wrong. Please, explain.
>>>
>>> If UseJVMCICompiler is false, then JVMCI is not being used by the VM as a jit but is only being used in hosted mode (e.g. by Truffle). The thinking was that hosted clients do not typically care about VM policies effecting the compilability of a method (e.g. whether it has a breakpoint etc). However, now that I reflect on this further, such clients should ignore this call altogether as they may be in a VM environment where both hosted and non-hosted compilations are being requested of JVMCI. With that in mind, I’ve updated the webrev to remove all mention and use of UseJVMCICompiler.
>>>
>>> -Doug
>>>
>>>>
>>>> On 2/14/17 12:11 PM, Doug Simon wrote:
>>>>> Please review this fix for a rather embarrassing bug I committed as part of JDK-8172733.
>>>>>
>>>>> http://cr.openjdk.java.net/~dnsimon/8174961/webrev/
>>>>> https://bugs.openjdk.java.net/browse/JDK-8174961
>>>>>
>>>>> -Doug
>>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8174961: [JVMCI] incorrect implementation of isCompilable

Doug Simon @ Oracle
Thanks for the thorough review.

> On 14 Feb 2017, at 23:01, Vladimir Kozlov <[hidden email]> wrote:
>
> Good.
>
> Thanks,
> Vladimir
>
> On 2/14/17 12:59 PM, Doug Simon wrote:
>>
>>> On 14 Feb 2017, at 21:47, Vladimir Kozlov <[hidden email]> wrote:
>>>
>>> Should you also remove flag check in previous lines in the test?:
>>>
>>>        boolean isCompilable = CompilerToVMHelper.isCompilable(method);
>>>        boolean expected = UseJVMCICompiler || WB.isMethodCompilable(aMethod);
>>>
>>
>> Yes, you’re right - webrev is updated with this change. Thanks for catching this oversight!
>>
>> -Doug
>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 2/14/17 12:40 PM, Doug Simon wrote:
>>>>
>>>>> On 14 Feb 2017, at 21:28, Vladimir Kozlov <[hidden email]> wrote:
>>>>>
>>>>> I did not get the condition. Why a method is *compilable* if JVMCICompiler is not used? Should it be
>>>>>
>>>>> UseJVMCICompiler && !method->is_not_compilable(
>>>>>
>>>>> Then comment is wrong. Please, explain.
>>>>
>>>> If UseJVMCICompiler is false, then JVMCI is not being used by the VM as a jit but is only being used in hosted mode (e.g. by Truffle). The thinking was that hosted clients do not typically care about VM policies effecting the compilability of a method (e.g. whether it has a breakpoint etc). However, now that I reflect on this further, such clients should ignore this call altogether as they may be in a VM environment where both hosted and non-hosted compilations are being requested of JVMCI. With that in mind, I’ve updated the webrev to remove all mention and use of UseJVMCICompiler.
>>>>
>>>> -Doug
>>>>
>>>>>
>>>>> On 2/14/17 12:11 PM, Doug Simon wrote:
>>>>>> Please review this fix for a rather embarrassing bug I committed as part of JDK-8172733.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~dnsimon/8174961/webrev/
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8174961
>>>>>>
>>>>>> -Doug
>>>>>>
>>>>
>>