RFR(L): JEP165: Compiler Control

Nils Eliasson nils.eliasson at oracle.com
Wed Oct 7 09:47:32 UTC 2015


Hi Zoltan,

It was not the intention to remove. I found a bug - I had spelled 
"DisableIntrinsic" wrong in the flag table in compilerDirectives.hpp. 
The stringly typed option commands didn't complain and I missed it.

Thanks for finding the bug!

Best regards,
Nils Eliasson


On 2015-10-07 09:44, Zoltán Majó wrote:
> Hi Nils,
>
>
> thank you for the clarifications! Please see my comments below.
>
> On 10/06/2015 06:57 PM, Nils Eliasson wrote:
>> 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 :)
>
> I'm afraid that the functionality is indeed removed. Here is a small 
> experiment to prove that.
>
> I use the following test program:
>
> class MathTest {
>     public static double test1() {
>         return Math.sin(3.14);
>     }
>
>     public static double test2() {
>         return Math.sin(3.14);
>     }
>
>     public static void main(String args[]) throws Exception {
>         double value = 0.0;
>
>         for (int i = 0; i < 20000; i++) {
>             value = test1();
>         }
>
>         for (int i = 0; i < 20000; i++) {
>             value = test2();
>         }
>     }
> }
>
>
> I execute MathTest with JDK9-b83 and also a recent build patched with 
> your changes. I use the following command line:
>
>  -XX:CompileCommand=option,MathTest::test2,ccstr,DisableIntrinsic,_dsin
>
> That means that the Math.sin intrinsic can be inlined into every 
> method except MathTest.test2.
>
> With JDK9-b83 I get the expected behavior (Math.sin is inlined into 
> test1 but not into test2)
>
>     124    1    b  3       MathTest::test1 (7 bytes)
>                               @ 3   java.lang.Math::sin (5 bytes) 
> intrinsic
>     131    2    b  3       MathTest::test2 (7 bytes)
>                               @ 3   java.lang.Math::sin (5 bytes)
>                 @ 1  java/lang/StrictMath::sin (not loaded)   not 
> inlineable
>     133    3     n 0       java.lang.StrictMath::sin (native) (static)
>
> With your changes I get:
>
>     143    1    b  3       MathTest::test1 (7 bytes)
>                               @ 3   java.lang.Math::sin (5 bytes) 
> intrinsic
>     148    2    b  3       MathTest::test2 (7 bytes)
>                               @ 3   java.lang.Math::sin (5 bytes) 
> intrinsic
>
> That is, Math.sin is inlined into both test1 and test2, even though 
> the per-method DisableIntrinsc flag forbids that.
>
> Is it intended that you remove this functionality and if yes, why do 
> you remove it?
>
> Thank you and best regards,
>
>
> Zoltan
>
>
>
>>
>> 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
>>>>
>>>
>>
>



More information about the hotspot-compiler-dev mailing list