RFR(L): JEP165: Compiler Control

Zoltán Majó zoltan.majo at oracle.com
Wed Oct 7 07:44:28 UTC 2015


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