[9] RFR(M): 8078554: Compiler: implement ranges (optionally constraints) for those flags that have them missing
Tobias Hartmann
tobias.hartmann at oracle.com
Wed Oct 7 13:38:20 UTC 2015
Hi Zoltan,
I had a look at your changes and just spotted some minor things:
globals_sparc.hpp:
- I think there is a '\' missing in line 119
globals_x86.hpp:
- Isn't this also a compiler flag we should add range checks for?
136 product(uintx, RTMRetryCount, 5,
commandLineFlagConstraintsCompiler.cpp:
- I think there is a "rule" that the include statements should be in alphabetical order
- the indentation is wrong here:
179 return Flag::VIOLATES_CONSTRAINT;
Best,
Tobias
On 06.10.2015 13:45, Zoltán Majó wrote:
> Hi Roland,
>
>
> thank you for the feedback!
>
> On 10/02/2015 03:55 PM, Roland Westrelin wrote:
>> Hi Zoltan,
>>
>>> Webrev:http://cr.openjdk.java.net/~zmajo/8078554/
>> c2_globals.hpp
>>
>> That one is not correct:
>> 461 product(intx, MaxNodeLimit, 80000, \
>> 462 "Maximum number of nodes") \
>> 463 range(1000, 80000) \
>>
>> I think the upper bound should be max_juint
>
> You are right that the limit of 80'000 is too conservative. But max_j*u*int as an upper bound would cause an overflow when parsing the flag's value, because on 32-bit machines intx is a 32-bit signed integer.
>
> Using max_jint instead of max_j*u*int as an upper bound would not cause an overflow at parse time. However, in Parse::do_call() the maximum node limit is increased by 3 times for jsr292 users
>
> C->set_max_node_limit(3*MaxNodeLimit);
>
> If MaxNodeLimit == max_jint, this expression will overflow, I think.
>
> So I set the limit to (max_jint / 3) in the updated webrev.
>
> If we would set MaxNodeLimit to max_j*u*int / 3 (instead of max_jint / 3), the expression 3 * MaxNodeLimit would overflow as well. Changing the type of the flag from intx to uintx could let use use max_j*u*int / 3 as an upper bound, but that is most likely not worth the effort.
>
>> 699 product(intx, LiveNodeCountInliningCutoff, 40000, \
>> 700 "max number of live nodes in a method") \
>> 701 range(0, max_juint / 8) \
>>
>> Out of curiosity why max_juint / 8 (not that it makes much of a difference)?
>
> In Compile::inline_incrementally, the 80% of LiveNodeCountInliningCutoff is computed the following way:
>
> if (low_live_nodes < (uint)LiveNodeCountInliningCutoff * 8 / 10) {
>
> If LiveNodeCountInliningCutoff == max_juint, we'd have an overflow because of the multiplication by 8.
>
>> arguments.cpp
>>
>> 1099 Tier3InvokeNotifyFreqLog = 0;
>> 1100 Tier4InvocationThreshold = 0;
>>
>> Why that change?
>
> I proposed that change because I misread the code. I reverted that change and also changed the range of all Tier*FreqLog flags from range(1, 30) to range(0, 30).
>
>> globals.hp
>>
>> 2870 product_pd(uintx, TypeProfileLevel, \
>> 2871 "=XYZ, with Z: Type profiling of arguments at call; " \
>> 2872 "Y: Type profiling of return value at call; " \
>> 2873 "X: Type profiling of parameters to methods; " \
>> 2874 "X, Y and Z in 0=off ; 1=jsr292 only; 2=all methods") \
>> 2875 range(0, 222)
>>
>> Legal values are 0, 1, 2, 10, 11, 12, 100, 101, 102, 110, 111, 112 etc.
>>
>> 70 is not for instance. So range(0, 222) is incorrect.
>
> I agree. I removed the range check and implemented a constraint function instead (TypeProfileLevelConstraintFunc).
>
>> 2877 product(intx, TypeProfileArgsLimit, 2, \
>> 2878 "max number of call arguments to consider for type profiling") \
>> 2879 range(0, 16) \
>>
>> 2880 \
>> 2881 product(intx, TypeProfileParmsLimit, 2, \
>> 2882 "max number of incoming parameters to consider for type profiling"\
>> 2883 ", -1 for all") \
>> 2884 range(-1, 64)
>>
>> Why 16 and 64?
>
> These are the largest values that work on all platforms we support.
>
> Here is the updated webrev: http://cr.openjdk.java.net/~zmajo/8078554/webrev.01/
>
> I repeated the testing with JPRT. I also executed the currently disabled TestOptionsWithRanges.java test on all platforms we support. All tests pass.
>
> Thank you and best regards,
>
>
> Zoltan
>
>> Roland.
>
More information about the hotspot-compiler-dev
mailing list