RFR(M): 8007288: Additional WB API for compiler's testing

Wed Apr 3 13:23:30 PDT 2013

 >>> I can add explicit checking for equality to 'CompLevel_any'.
 > should i add this check?

I don't think it is needed. May be comment.

You convinced me about changes in CompilationPolicy::can_be_compiled().

Now, why you need new changes in compilationPolicy.hpp? It was not in 
previous webrev. Yes, it may be correct but why?

Vladimir

On 4/3/13 12:35 PM, Igor Ignatyev wrote:
>  >>> compileBroker.cpp - add assert() to check that.
>  >> If I add it, WB.enqueueMethodForCompilation() will trigger this assert,
>  >> e.g. in test 'compiler/whitebox/EnqueueMethodForCompilationTest'.
>  >
>  > Which means the test introduces the case which never happens in normal
>  > execution. I am not sure it is good or bad.
>  > You can relax assert with WhiteBoxAPI check. I really want this assert
>  > for normal execution.
> i added assert and changed 'NonTieredCompPolicy::initial_compile_level'
> since it return 'CompLevel_full_profile' instead of
> 'CompLevel_full_optimization' for non-tiered C2 with Xcomp. sorry, i
> missed this it due to poor preprocessor in my brain.
>
>>>> compilationPolicy.cpp - I don't think the change is correct since
>>>> is_compile(CompLevel_any) returns false.
>>> 'comp_level == CompLevel_any' is the same that 'comp_level ==
>>> CompLevel_all' and this situation is processed in
>>> compilationPolicy.cpp:124, so I think that it's correct, isn't it?
>>> I can add explicit checking for equality to 'CompLevel_any'.
> should i add this check?
>
>> You are right about CompLevel_any. What about next check in
>> m->is_not_compilable():
>>
>>    if (is_method_handle_intrinsic())
>>      return !is_synthetic();  // the generated adapters must be compiled
>>
> even if CompilationPolicy::can_be_compiled() return 'true' for invalid
> comp_level, method won't be compiled since
> CompileBroker::compiler(comp_level) will be 'NULL' in
> CompileBroker::compile_method():
>
> AbstractCompiler *comp = CompileBroker::compiler(comp_level);
> if (comp == NULL || !comp->can_compile_method(method) ||
>      compilation_is_prohibited(method, osr_bci, comp_level)) {
>    return NULL;
> }
>
> updated webrev: http://cr.openjdk.java.net/~iignatyev/8007288/webrev.02/
>
> Best regards,
> Igor Ignatyev
>
> On 04/02/2013 11:19 PM, Vladimir Kozlov wrote:
>> On 4/2/13 1:49 AM, Igor Ignatyev wrote:
>>>  > compileBroker.cpp - add assert() to check that.
>>> If I add it, WB.enqueueMethodForCompilation() will trigger this assert,
>>> e.g. in test 'compiler/whitebox/EnqueueMethodForCompilationTest'.
>>
>> Which means the test introduces the case which never happens in normal
>> execution. I am not sure it is good or bad.
>> You can relax assert with WhiteBoxAPI check. I really want this assert
>> for normal execution.
>>
>>>
>>>  > compilationPolicy.cpp - I don't think the change is correct since
>>>  > is_compile(CompLevel_any) returns false.
>>> 'comp_level == CompLevel_any' is the same that 'comp_level ==
>>> CompLevel_all' and this situation is processed in
>>> compilationPolicy.cpp:124, so I think that it's correct, isn't it?
>>> I can add explicit checking for equality to 'CompLevel_any'.
>>
>> You are right about CompLevel_any. What about next check in
>> m->is_not_compilable():
>>
>>    if (is_method_handle_intrinsic())
>>      return !is_synthetic();  // the generated adapters must be compiled
>>
>> Thanks,
>> Vladimir
>>
>>>
>>>> Could you rename setForceInlineMethod() to
>>>> testSetForceInlineMethod() to
>>>> be more clear what it does?
>>> I will rename setForceInlineMethod() and setDontInlineMethod().
>>>
>>> Best regards,
>>> Igor Ignatyev
>>>
>>> On 04/01/2013 09:30 PM, Vladimir Kozlov wrote:
>>>> compileBroker.cpp - add assert() to check that.
>>>>
>>>> compilationPolicy.cpp - I don't think the change is correct since
>>>> is_compile(CompLevel_any) returns false.
>>>>
>>>> Could you rename setForceInlineMethod() to
>>>> testSetForceInlineMethod() to
>>>> be more clear what it does?
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 4/1/13 7:15 AM, Igor Ignatyev wrote:
>>>>> test 'compiler/whitebox/EnqueueMethodForCompilationTest' has failed
>>>>> during testing, because 'CompilationPolicy::can_be_compiled()' returns
>>>>> 'true' for invalid comp_level (less that -1 or great that 4).
>>>>>
>>>>> i added method 'is_compile' to check whether a value is valid level of
>>>>> compilation and rerun jprt.
>>>>>
>>>>> in new results:
>>>>>    - compiler/whitebox/IsMethodCompilableTest failed on on *-i586-*-c1
>>>>> due to JDK-8007270
>>>>>    - timeouts in other compiler's tests:
>>>>>      * compiler/6663848/Tester
>>>>>      * compiler/6711100/Test
>>>>>      * compiler/6792161/Test6792161
>>>>>      * compiler/6863420/Test
>>>>>      * compiler/6863420/Test
>>>>>      * compiler/6901572/Test
>>>>>      * compiler/7100757/Test7100757
>>>>>      * compiler/7119644/TestFloatVect
>>>>>      * compiler/7196199/Test7196199
>>>>>      * compiler/7199742/Test7199742
>>>>>
>>>>> updated webrev:
>>>>> http://cr.openjdk.java.net/~iignatyev/8007288/webrev.01/
>>>>>
>>>>> Best regards,
>>>>> Igor Ignatyev
>>>>>
>>>>> On 03/29/2013 01:52 AM, David Chase wrote:
>>>>>> (Not a reviewer)
>>>>>>
>>>>>> Could you run these tests on the "east" JPRT queue and see if they
>>>>>> have any problems there?
>>>>>> We seem to have some surprisingly slow machines there, and that has
>>>>>> caused problems with other tests.
>>>>>>
>>>>>> David
>>>>>>
>>>>>>
>>>>>> On 2013-03-28, at 5:41 PM, Igor Ignatyev <igor.ignatyev at oracle.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Please review the patch.
>>>>>>>
>>>>>>> 1. added new functions to WhiteBox:
>>>>>>>   public native boolean setForceInlineMethod(Method method, boolean
>>>>>>> value);
>>>>>>>   public native boolean enqueueMethodForCompilation(Method method,
>>>>>>> int compLevel);
>>>>>>>   public native void    clearMethodState(Method method);
>>>>>>> 2. related changes in not whitebox code:
>>>>>>>   - added 'Method::clear_not_*_compilable()' and corresponded
>>>>>>> 'AccessFlags::clear_not_*_compilable()'
>>>>>>>   - added 'MethodData::init()' - reset method data into original
>>>>>>> state. reset all counters, flags
>>>>>>> 3. removed useless code:
>>>>>>>   - 'if (!TieredCompilation) { comp_level = CompLevel_highest_tier;
>>>>>>> }' in CompileBroker::compile_method() -- there is no method calls w/
>>>>>>> comp_level != CompLevel_highest_tier in non-tiered
>>>>>>>   - 'MethodData::initialize(methodHandle method)' -- declaration w/o
>>>>>>> definition
>>>>>>> 4. added tests for new WB functions
>>>>>>>
>>>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8007288/webrev.00/
>>>>>>> jbs: https://jbs.oracle.com/bugs/browse/JDK-8007288
>>>>>>> --
>>>>>>> Best regards,
>>>>>>> Igor Ignatyev
>>>>>>