RFR(L): JEP165: Compiler Control

[Nils Eliasson]

Hi Zoltan,

On 2015-10-06 15:55, Zoltán Majó wrote:
> Hi Nils,
>
>
> Currently, there are two ways the the DisableIntrinsic flag can be 
> used to disable intrinsics.
>
> 1) Intrinsics can be disabled globally, that is, the compilers are not 
> allowed to "use" (e.g., inline) the intrinsic in any calling context. 
> For example:
>
> -XX:DisableIntrinsic=_hashCode,_getClass
>
> disables two intrinsics globally.
>
> 2) Intrinsics can be disabled on a per-method level, that is, the 
> compiler is not allowed to use the intrinsic if it is called from a 
> specific method, but the compiler is still allowed to use the 
> intrinsic otherwise. For example:
> overri
>  -XX:CompileCommand=option,aClass::aMethod,ccstr,DisableIntrinsic,_hashCode 
>
>
> disables intrinsification of _hashCode if it is called from 
> aClass::aMethod (but not for any other call site of _hashCode).
>
> It seems to me that your changes preserve (1) but eliminate (2). Could 
> you please explain to me why you remove the functionality of disabling 
> intrinsics on a per-method level?

It isn't removed :)

The option is as you say available as a VM flag and as an per method 
CompileCommand option. Compiler Directives has full compatibility with 
this with one small exception.

1) The global vm flag "-XX:DisableIntrinsic=_hashCode,_getClass " is 
added as the default value to all directives.
2) CompileCommand option " 
-XX:CompileCommand=option,aClass::aMethod,ccstr,DisableIntrinsic,_hashCode 
" overrides this if the method is matching. That matching is taking 
place in compilecommand_compatibility_init(). This happens after a 
directive is choosen because the directive and compile command may have 
different matches.
3) Any CompilerDirective that set this flag will override both above.

The difference is that you where able to forbid the union of the 
intrinsics in the vmflag and the one from the compilecommand. Now 
CompileCommand overrides the vmflag. This is how all the other options 
that are available as vmflags and compilecommand works. I haven't seen 
this in any test or use case - but if this is a problem I could make a 
custom workaround for this option that appends the two sets. 
CompilerDirectives will always override since you want to be able to 
remove any flag that is set.

The table at the beginning of compilerDirectives.hpp declares all 
options, their default values (sometimes vmflags, sometimes constants) 
and any compilecommand that must be checked.

>
> Also, currently there are some comments describing the way the 
> DisableIntrinsic flag works:
> - abstractCompiler.hpp: line 70--98: Please update the description to 
> match the way DisableIntrinsic works with your changes;
> - vmSymbols.cpp: line 420; Please update the comment. Also, please 
> also move part of that comment to the place where you check if 
> intrinsics are disabled, as the method is_disabled_by_flags() does not 
> check at the value of the DisableIntrinsic flag.

Ok, I'll check and update the comments.

Thanks for looking at my code!

Best regards,
Nils Eliasson

>
> 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
>>
>