RFR(L): JEP165: Compiler Control

[Nils Eliasson]

Hi Zoltan,

See below.

On 2015-10-07 10:09, Zoltán Majó wrote:
> Hi Nils,
>
>
> thank you for clarification. I think in my previous email I did not 
> formulate my question clearly enough.
>
> What I meant is the following: Previously, ciMethod::has_option_value 
> was used to determine if there is a per-method flag value available 
> for a method. You have replaced the usages of 
> ciMethod::has_option_value() with querying a directive directly. For 
> example:
>
> src/share/vm/opto/idealGraphPrinter.cpp
> -  method->has_option_value("PrintIdealGraphLevel", 
> ideal_graph_level); // update value with per-method value (if available)
> +  return C->directive()->IGVPrintLevelOption >= level;
>
> src/share/vm/opto/compile.cpp
> -  method_has_option_value("MaxNodeLimit", _max_node_limit);
> +  _max_node_limit = _directive->MaxNodeLimitOption;
>
> src/share/vm/opto/superword.cpp
> - _phase->C->method()->has_option_value("VectorizeDebug", 
> _vector_loop_debug);
> +    _vector_loop_debug = phase->C->directive()->VectorizeDebugOption;
>
>
> The only location in the source code that you still use 
> has_option_value() is:
>
> src/share/vm/compiler/compileBroker.cpp
> -  if (CompilerOracle::should_exclude(method, quietly)
> -      || (CompilerOracle::has_option_value(method, 
> "CompileThresholdScaling", scale) && scale == 0)) {
> +  if (excluded || (CompilerOracle::has_option_value(method, 
> "CompileThresholdScaling", scale) && scale == 0)) {
>
> Could you please explain why you use has_option_value() at this source 
> location instead of a directive (as at all other places, e.g, in the 
> case of PrintIdealGraphLevel, MaxNodeLimit, and VectorizeDebug)?

Yes. The intention was to use directives everywhere where 
option-commands are used. However - the "CompileThresholdScaling" (and 
the lock eliding options) is also accessed from outside the compilers. 
(That means a lot of the efficiency gains of having directives is lost.) 
I didn't have the time to test and verify the performance of that, so I 
have left it out. I will address this in the future as a small update.

Best regards,
Nils

>
> Thank you and best regards,
>
>
> Zoltan
>
> On 10/06/2015 07:06 PM, Nils Eliasson wrote:
>> Hi Zoltan,
>>
>> All the CompileCommand options work in the same way as described in 
>> the intrinsics-mail. In compilerDirectives.hpp the flags are declared 
>> with their CompileCommand equivalent. And in 
>> compilecommand_compatibility_init the directives may be updated with 
>> these flags.
>>
>> Take a look at 
>> test/compiler/compilercontrol/TestCompilerDirectivesCompatibilityCommandOn.java 
>> for an example of how it can work. First these are set on the 
>> commandline "-XX:-PrintAssembly -XX:CompileCommand=print,*.*", then 
>> in the test a directive is loaded that turns it off. Later the 
>> directive is removed. The result is verified at each step.
>>
>> I hope my explanation is understandable.
>>
>> Best regards,
>> Nils Eliasson
>>
>>
>>
>>
>> On 2015-10-06 16:16, Zoltán Majó wrote:
>>> Hi,
>>>
>>>
>>> there is another question that comes to my mind.
>>>
>>> For command-line flags that allow per-method usage you propose to 
>>> get the flag's value from the directive instead of getting the value 
>>> using ciMethod()::has_option_value(). For example, you propose to use
>>>
>>> _vector_loop_debug = directive()->VectorizeDebugOption;
>>>
>>> instead of
>>>
>>> method()->has_option_value("VectorizeDebug", _vector_loop_debug);
>>>
>>> You propose to do this for almost all flags (e.g., VectorizeDebug, 
>>> DisableIntrinsic, MaxNodeLimit, PrintIdealGraphLevel) except one, 
>>> CompileThresholdScaling. Could you please explain why?
>>>
>>> Thank you and best regards,
>>>
>>>
>>> Zoltan
>>>
>>> On 10/02/2015 04:10 PM, Nils Eliasson wrote:
>>>> Hi all,
>>>>
>>>> A new webrev with all the feedback from reviewers and others.
>>>>
>>>> http://cr.openjdk.java.net/~neliasso/8046155/webrev.04/
>>>>
>>>> - name changes as requested by christian
>>>> - various small bug fixes
>>>> - New tests for sanity testing flags and their behaviour with vm 
>>>> flags and compile commands present
>>>>
>>>> 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
>>>>
>>>
>>
>