[9] RFR(S): 8059847: complement JDK-8055286 and JDK-8056964 changes

[Zoltán Majó]

Hi Vladimir,


thank you for your feedback!

On 10/08/2014 05:27 PM, Vladimir Kozlov wrote:
> You don't need /othervm if there  are no additional flags.

OK, I removed it.

> Instead of /startup/ I would suggest /oracle/ subdir name since we 
> testing code in compilerOracle.

OK, I moved the test to the 'oracle' subdirectory.

> 42     // Type (1) is used to check if a flag "someflag" is enabled for a
> 43     // method.
>
> I would say:
>
> 42     // Type (1) is used to enable a boolean flag for a method.

I updated the comment in the test (and also in compilerOracle.cpp on 
line 732).

> Should it be "bool,MyBoolOption,100"?:
>
>  135             // wrong value for bool flag
>  136 "-XX:CompileCommand=option,Test::test,bool,MyBoolOption,intx,100",

I changed the test to what you've suggested.

> Same problem with next test which does not match the comment:
>
> 140             // intx flag name missing
> 141 
> "-XX:CompileCommand=option,Test::test,bool,MyBoolOption,false,intx,100",

I changed this test as well.

I've also added three new tests to TYPE_2_INVALID_ARGUMENTS. The new 
tests are very similar to the ones in webrev.01.

> Please, separate next checks to be clear which one is incorrect:
>
>  179         if (TYPE_1_ARGUMENTS.length != 
> TYPE_1_EXPECTED_OUTPUTS.length
>  180             || TYPE_2_ARGUMENTS.length != 
> TYPE_2_EXPECTED_OUTPUTS.length) {
>  181             throw new RuntimeException("Test is set up 
> incorrectly: length of arguments and expected outputs does not match.");
>  182         }

I separated the checks and updated the error messages.

Here is the new webrev:

http://cr.openjdk.java.net/~zmajo/8059847/webrev.02/

Thank you!

Best regards,


Zoltan

>
> Thanks,
> Vladimir
>
> On 10/8/14 7:22 AM, Zoltán Majó wrote:
>> Hi David and Vladimir,
>>
>>
>> thank you for your feedback.
>>
>> On 10/07/2014 10:46 PM, Vladimir Kozlov wrote:
>>> On 10/7/14 11:06 AM, David Chase wrote:
>>>
>>>> Do we need to add a test (? what would it look like?) to ensure 
>>>> that it works?
>>>
>>> Yes, please, add a regression test.
>>
>> I added a regression test 
>> ('compiler/startup/CheckCompileCommandOption.java'). Here is the 
>> updated webrev:
>>
>> http://cr.openjdk.java.net/~zmajo/8059847/webrev.01/
>>
>> Thank you and best regards,
>>
>>
>> Zoltan
>>
>>>
>>> Thanks,
>>> Vladimir
>>>
>>>>
>>>> David
>>>>
>>>> On 2014-10-07, at 1:53 PM, Zoltán Majó <zoltan.majo at oracle.com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>>
>>>>> please review the following small patch.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8059847
>>>>>
>>>>> Problem: CompileCommand=option does not support flags of type double.
>>>>>
>>>>> Solution: Add support for flags of type double. Output:
>>>>>
>>>>> java 
>>>>> -XX:CompileCommand=option,Test::test,ccstrlist,MyListOption,_foo,_bar
>>>>> -XX:CompileCommand=option,Test::test,ccstr,MyStrOption,_foo
>>>>> -XX:CompileCommand=option,Test::test,bool,MyBoolOption,false
>>>>> -XX:CompileCommand=option,Test::test,intx,MyIntxOption,-1 
>>>>> -XX:CompileCommand=option,Test::test,uintx,MyUintxOption,1
>>>>> -XX:CompileCommand=option,Test::test,MyFlag 
>>>>> -XX:CompileCommand=option,Test::test,double,MyDoubleOption,1.123 
>>>>> -version
>>>>>
>>>>> CompilerOracle: option Test.test const char* MyListOption = '_foo 
>>>>> _bar'
>>>>> CompilerOracle: option Test.test const char* MyStrOption = '_foo'
>>>>> CompilerOracle: option Test.test bool MyBoolOption = false
>>>>> CompilerOracle: option Test.test intx MyIntxOption = -1
>>>>> CompilerOracle: option Test.test uintx MyUintxOption = 1
>>>>> CompilerOracle: option Test.test bool MyFlag = true
>>>>> CompilerOracle: option Test.test double MyDoubleOption = '1.123000'
>>>>> java version "1.9.0-ea"
>>>>> Java(TM) SE Runtime Environment (build 1.9.0-ea-b33)
>>>>> Java HotSpot(TM) 64-Bit Server VM (build 1.9.0-fastdebug-internal, 
>>>>> mixed mode)
>>>>>
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~zmajo/8059847/webrev.00/
>>>>>
>>>>> Testing: JPRT
>>>>>
>>>>> Thank you and best regards,
>>>>>
>>>>>
>>>>> Zoltan
>>>>>
>>>>
>>