[9] RFR(S): 8055286: Extend CompileCommand=option to handle numeric parameters

[Zoltán Majó]

Hi Vladimir,


thank you for the feedback.

On 08/22/2014 07:11 PM, Vladimir Kozlov wrote:
> Sometimes we specify flag which is not declared as global flag in 
> globals.hpp, for example NoRTMLockEliding. Please, don't remove such 
> ability:

I did not know that, thanks for pointing it out.

>
> +           Flag *declared_flag = Flag::find_flag(flag, strlen(flag));
>
> Flags specified in 'option' command do not affect global flags. We use 
> the same name only for convenience.

I removed checking if a flag is declared in globals.hpp.

But I think it is important that the type of a flag specified by the 
user matches the type expected by the compiler (otherwise the compiler 
could read random values at runtime). The type of a flag is therefore 
recorded in TypedMethodOptionMatcher and it is checked at runtime (line 
381). If the two types do not match for a method m(), the default value 
of the flag is used for that method.

> Could you move new option code from parse_from_line() method to 
> separate method()?

Yes, I did that. Please see the scan_flag_and_value() function.

> Also parts of that code also could be factored out, like codes which 
> process 'intx' and 'uintx' values - they are very similar.

I tried that, but it is not straightforward, as on some platforms 
sscanf() does not support format strings created at runtime. Without a 
common sscanf() call, the two branches are too little similar, but I 
might be wrong on that.

So I decided to leave the code processing intx and uintx values 
unchanged. I hope that factoring out functionality into the 
scan_flag_and_value() function made the code already more readable.

> Use 'Klass' in second line too:
> +       // (1) CompileCommand=option,Klass::method,flag
> +       // (2) CompileCommand=option,KLASS::method,type,flag,value

Changed that as well.

Here is the updated webrev: 
http://cr.openjdk.java.net/~zmajo/8055286/webrev.01/

I ran the JPRT tests again, all pass.

Thank you and best regards


Zoltan

> Thanks,
> Vladimir
>
> On 8/22/14 2:12 AM, Zoltán Majó wrote:
>> Hi,
>>
>>
>> please review the following patch.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8055286
>>
>> Problem: Currently, CompileCommand=option handles only flags of type 
>> bool. CompileCommand=option should be extended to
>> handle numeric types as well.
>>
>> Solution: This patch adds support for processing flags of type intx 
>> and uintx (in addition flags of type bool). Support
>> for flags of type ccstr and ccstrlist is not added by this patch; we 
>> can add support for those types when it is needed.
>>
>> Webrev: http://cr.openjdk.java.net/~zmajo/8055286/
>>
>> Testing: JPRT, manual testing
>>
>> Thank you and best regards,
>>
>>
>> Zoltan
>>