RFR(L): JEP165: Compiler Control

[Zoltán Majó]

Hi Nils,


On 10/13/2015 07:16 PM, Nils Eliasson wrote:
> Hi Zoltan,
>
> Thanks for the feedback.
>
> The JDK webrev is here:
> http://cr.openjdk.java.net/~neliasso/8046155/webrev_jdk.02/

this looks good to me.

Thank you and best regards,


Zoltan

>
> I have fixed the comment too and will update the hotspot webrev shortly.
>
> Thanks,
> Nils
>
> On 2015-10-08 15:33, Zoltán Majó wrote:
>> Hi Nils,
>>
>>
>> On 10/08/2015 02:10 PM, Nils Eliasson wrote:
>>> Hi Zoltan,
>>>
>>> Thanks for the repro. Found a bug where the base directives got 
>>> corrupted. Added test cases to the 
>>> TestCompilerDirectivesCompatibility* which will cover that.
>>
>> Thank you for fixing the problem with the DisableIntrinsic flag (the 
>> flag works now as expected).
>>
>> I've spotted two other small problems:
>>
>> 1) src/share/vm/compiler/abstractCompiler.hpp
>>
>>   72   //  - the intrinsic is enabled (by using the appropriate 
>> command-line flag
>>   73   //    , the command-line compile ommand, or a compiler directive)
>>
>> The comma on line 73 should be at the end of line 72.
>>
>> 2) In your JDK changes (I've looked at webrev_jdk.01, as I think 
>> there is no newer version): You have not declared the 
>> sun.hotspot.WhiteBox::shouldPrintAssembly(Ljava/lang/reflect/Executable;)Z 
>> method. As a result the VM is complaining if the WhiteBox API is 
>> enabled:
>>
>> Warning: 'NoSuchMethodError' on register of 
>> sun.hotspot.WhiteBox::shouldPrintAssembly(Ljava/lang/reflect/Executable;)Z
>>
>>
>> Thank you and best regards,
>>
>>
>> Zoltan
>>
>>>
>>> New webrev:
>>> http://cr.openjdk.java.net/~neliasso/8046155/webrev.10/
>>>
>>> Best regards,
>>> Nils
>>>
>>>
>>> On 2015-10-07 15:27, Zoltán Majó wrote:
>>>> Hi Nils,
>>>>
>>>>
>>>> On 10/07/2015 12:38 PM, Nils Eliasson wrote:
>>>>> Hi all,
>>>>>
>>>>> Latest webrev including fixes after comments from Chris and Zoltan:
>>>>> http://cr.openjdk.java.net/~neliasso/8046155/webrev.09/
>>>>
>>>> thank you for the updated version. It seems, however, that the 
>>>> DisableIntrinsic flag does not work correctly on the per-method level.
>>>>
>>>> If I execute the MathTest program I've sent out before with the 
>>>> command-line option
>>>>
>>>> -XX:CompileCommand=option,MathTest::test1,ccstr,DisableIntrinsic,_dsin
>>>>
>>>> with jdk9-b83, I get the expected behavior (Math.sin is inlined 
>>>> into test2 but not into test1):
>>>>
>>>>     128    1    b  3       MathTest::test1 (7 bytes)
>>>>                               @ 3   java.lang.Math::sin (5 bytes)
>>>>                                 @ 1 java/lang/StrictMath::sin (not 
>>>> loaded)   not inlineable
>>>>     130    2     n 0       java.lang.StrictMath::sin (native) (static)
>>>>     132    3    b  3       MathTest::test2 (7 bytes)
>>>>                               @ 3   java.lang.Math::sin (5 bytes) 
>>>> intrinsic
>>>>
>>>>
>>>> If I execute the test case with a locally-built jdk patched with 
>>>> webrev.09, I get an erronous behavior (Math.sin is not inlined into 
>>>> test2 although it should be):
>>>>
>>>>     133    1    b  3       MathTest::test1 (7 bytes)
>>>>                               @ 3   java.lang.Math::sin (5 bytes)
>>>>                                 @ 1 java/lang/StrictMath::sin (not 
>>>> loaded)   not inlineable
>>>>     136    2     n 0       java.lang.StrictMath::sin (native) (static)
>>>>     138    3    b  3       MathTest::test2 (7 bytes)
>>>>                               @ 3   java.lang.Math::sin (5 bytes)
>>>>                                 @ 1 java.lang.StrictMath::sin (0 
>>>> bytes)   native method
>>>>
>>>> Could you please check why that happens?
>>>>
>>>> Thank you and best regards,
>>>>
>>>>
>>>> Zoltan
>>>>
>>>>
>>>>
>>>>>
>>>>> Regards,
>>>>> //Nils
>>>>>
>>>>>
>>>>>
>>>>> On 2015-09-22 19:21, Nils Eliasson wrote:
>>>>> > Hi, > > This is the initial RFR for JEP165: Compiler Control. 
>>>>> This feature > enables runtime manageable, method dependent 
>>>>> compiler flags. > (Immutable for the duration of a compilation.) > 
>>>>> > The change includes: - A parser for the directives format (json 
>>>>> like) > including vmtests (json.cpp/hpp) - A component for 
>>>>> construction of > compiler directives (directivesParser.cpp/hpp) - 
>>>>> The directives > including the option definitions, default values 
>>>>> and compilecommand > relations (compilerDirectives.cpp/hpp) - 
>>>>> Diagnostic commands for > working with the directives - 
>>>>> installing, removing, printing - Lots > of small changes wherever 
>>>>> we access flags or legacy compilecommands > in the compiler > > 
>>>>> Notes: The feature is documented in the JEP > 
>>>>> (https://bugs.openjdk.java.net/browse/JDK-8046155). > > Currently 
>>>>> only a small amount of compiler flags are included in the > 
>>>>> change. The flags are a representative selection of different 
>>>>> types > targeting both compilers. All of them existed as 
>>>>> CompilerOracle > option commands. Two commands was not included in 
>>>>> the directives due > to time constraints - 
>>>>> CompilerThresholdScaling and UseRTMLocks. Both > are accessed from 
>>>>> runtime (outside any compiler) and requires some > special 
>>>>> handling. (Solved but not implemented.) > > Full backwards 
>>>>> compatibility with CompileCommands is implemented but > can be 
>>>>> turned off with flag -XX:CompileCommandCompatibilty. Also meta > 
>>>>> handling the compatibility flag by supporting it in the directives 
>>>>> > (test feature). > > The change contain some rough edges that 
>>>>> will polished over the > coming days. > > JEP: 
>>>>> https://bugs.openjdk.java.net/browse/JDK-8046155 Hotspot webrev: > 
>>>>> http://cr.openjdk.java.net/~neliasso/8046155/webrev.01/ JDK 
>>>>> webrev: > 
>>>>> http://cr.openjdk.java.net/~neliasso/8046155/webrev_jdk.01/ > > 
>>>>> Please review! > > Best regards, Nils Eliasson
>>>>>
>>>>>
>>>>
>>>
>>
>