RFR(M): 8223051: support loops with long (64b) trip counts
John Rose
john.r.rose at oracle.com
Fri May 29 08:11:00 UTC 2020
On May 28, 2020, at 6:19 AM, Roland Westrelin <rwestrel at redhat.com> wrote:
>
>> Maybe someone else will have comments 0n that, but assuming
>> that is more or less unchanged, this change set for 8223051 on
>> top still looks good, taking into account renaming of min/max
>> factories.
>>
>> After you post an updated webrev.02 the review should be easy.
>> Tobias might wish to run some regression tests on the final changes.
>
> Thanks for the review. Here is the webrev with the min/max renaming and
> the hunk that was erroneously included in 8244504.
>
> http://cr.openjdk.java.net/~roland/8223051/webrev.02/
>
> Roland.
Good, same as before.
I noticed one more corner case that might be good to address.
+ // We can't iterate for more than max int at a time.
+ if (stride_con != (jint)stride_con || ABS(stride_con) >= max_jint) {
+ return false;
+ }
Should be:
+ // We can't iterate for more than max int at a time.
++ if (stride_con != (jint)stride_con || ABS(stride_con) * (1+MIN_ITER) >= max_jint) {
+ return false;
+ }
Where MIN_ITER is some constant that defines the minimum
number of iterations that the strip-mined int-loop should run.
If the stride is very large (nearly max_jint) then the int-loop
will only run once, or just a few times. I think MIN_ITER
should be at least 10.
I suggest hardwiring it to 10 locally in loopnode.cpp, and
making it a tunable parameter later on if we actually
run into trouble with it. But we won’t; nobody is going
to write loops with strides on the order of max_jint.
In fact, you can leave out this suggestion altogether,
if you are not comfortable with it, and we just take the
odd performance hit if someone does something that
strange.
Either way, I say, ship it!
— John
More information about the hotspot-compiler-dev
mailing list