Request for reviews (XXL): 5091921: Sign flip issues in loop optimizer

[Dmitry Samersoff]

Vladimir,

Thank you in advance for fixing it!

-Dmitry


On 2011-04-29 03:04, Vladimir Kozlov wrote:
> http://cr.openjdk.java.net/~kvn/5091921/webrev
>
> Fixed 5091921: Sign flip issues in loop optimizer
>
> Loop optimizer in Server VM does not take into account integer overflow
> when it generates code for loop limits calculation during counted loop
> construction, unrolling and range check elimination. As result generated
> code will behave incorrectly when an integer overflow happens.
>
> 1. Added limit check for Counted loops which, if failed, causes
> recompilation of the method without counted loop. It based on loop
> predication code we have already but we don't need to copy it after
> corresponding counted loop is created (this is why a new flag is passed
> to loop predicate copy methods).
>
> if (limit>= max_int-stride)
> uncommon_trap
> counted_loop init, limit, stride
>
> 2. Long arithmetic is used to calculate exact limit only when it is
> needed: empty loop elimination and predicated range check elimination.
> New ideal macro node LoopLimitNode is used but corresponding mach node
> is created only for 32-bit x86 to get better assembler code. Sparc and
> x64 have long registers so generated assembler is good enough without
> specialized mach node. Also delay LoopLimitNode transformation until all
> loop optimizations are done (by marking LoopLimitNode as macro node).
>
> 3. New limit after unroll calculated as:
>
> new_limit = limit-stride
> new_limit = (limit < new_limit) ? MININT : new_limit;
>
> instead of current expression:
>
> new_limit = init + (((limit-init)/stride)&(-2))*stride
>
> Added other checks to avoid limit value overflow during unroll. Also
> fully unroll loops with up to 3 trip count.
>
> 4. Added underflow check for normal compares in loops which are
> optimized out using range check elimination code (I also refactored the
> code):
>
> MIN_INT <= scale*I+offset < limit
>
> ----------------------------------------------
> These changes are put under flags to debug associated problems. I also
> allowed to print inlining decisions in product VM since PrintInlining is
> diagnostic flag now. New regression tests are added based on associated
> bug reports.
>
> These changes cause performance regression in benchmarks. Some of
> regression cases can't not be avoided but some we will address in a future.
>
> And finally I want to thank java VM team from HP who provided nice test
> cases and even suggested fix. I used their idea for unroll limit fix.


-- 
Dmitry Samersoff
Java Hotspot development team, SPB04
* There will come soft rains ...