RFR: 8330158: C2: Loop strip mining uses ABS with min int

[Vladimir Kozlov]

On Wed, 17 Apr 2024 10:45:04 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> This fixes 3 calls to ABS with a min int argument. I think all of them
> are harmless:
> 
> - in `PhaseIdealLoop::exact_limit()`, I removed the call to ABS. The
>   check is for a stride of 1 or -1.
>   
> - in `OuterStripMinedLoopNode::adjust_strip_mined_loop()`, for the
>   computation of `scaled_iters_long`, the stride is passed to `ABS()`
>   and then implicitly casted to long. I now cast the stride to long
>   before `ABS()`. For a min int stride, `LoopStripMiningIter * stride`
>   overflows the int range for all values of `LoopStripMiningIter`
>   except 0 or 1. Those values are handled early on in that method. So
>   for a min in stride:
>   ```
>   (jlong)scaled_iters != scaled_iters_long
>   ```
>   is always true and the method returns early.
>   
> - in `OuterStripMinedLoopNode::adjust_strip_mined_loop()`, the
>   computation of `short_scaled_iters` also calls `ABS()` with the
>   stride as argument. But the result of that computation is only used
>   if the test for:
>   ```
>   (jlong)scaled_iters != scaled_iters_long
>   ```
>   doesn't cause an early return of the method. I reordered statements
>   so the `ABS()` calls happens after that test which will cause an early
>   return if the stride is min int.

two comments

src/hotspot/share/opto/loopnode.cpp line 2969:

> 2967:   int scaled_iters = (int)scaled_iters_long;
> 2968:   if ((jlong)scaled_iters != scaled_iters_long) {
> 2969:     // Remove outer loop and safepoint (too few iterations)

Please put more extended comment here. What you have in PR description would be nice.

src/hotspot/share/opto/loopnode.cpp line 2973:

> 2971:     return;
> 2972:   }
> 2973:   int short_scaled_iters = LoopStripMiningIterShortLoop * ABS(stride);

So stride is not MIN_INT here but the expression still can overflow. Should we use `jlong` for expression and `short_scaled_iters`? `iter_estimate` is `jlong`.

-------------

PR Review: https://git.openjdk.org/jdk/pull/18813#pullrequestreview-2006487913
PR Review Comment: https://git.openjdk.org/jdk/pull/18813#discussion_r1569112810
PR Review Comment: https://git.openjdk.org/jdk/pull/18813#discussion_r1569123977