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