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