RFR: 8299975: Limit underflow protection CMoveINode in PhaseIdealLoop::do_unroll must also protect type from underflow [v5]

[Roland Westrelin]

On Fri, 20 Jan 2023 12:35:42 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> This is a change similar to Rolands https://github.com/openjdk/jdk/pull/4388 [JDK-8267399](https://bugs.openjdk.org/browse/JDK-8267399): After the limit is modified, its type is not specific enough. Roland added a `CastII` in `PhaseIdealLoop::do_range_check` to keep the type.
>> 
>> I have to do a similar thing for `PhaseIdealLoop::do_unroll` - but I can directly constrain the type of the `CMoveI` node:
>> We decrease the limit: `limit = limit - stride`. The subtraction makes an old limit type `int:<=3` underflow to `int`. But the `CMoveI` is already there to prevent the new limit from underflowing - it just does not manage that it's condition together with the two values it conditionally picks never underflow. And so the type information becomes too wide.
>> 
>> This is a problem if we have the following loop (simplified from regression test `test_simple`):
>> 
>>     static void test_simple(boolean flag) {
>>         int x = flag ? Integer.MIN_VALUE : 3;
>>         // x has type "int:min..3" == "<=3"
>>         // Leads to Peel and 2x Unroll
>>         for (int i = 0; i < x; i++) {
>>             iArr[i * 2] = 666 + i;
>>         }
>>     }
>> 
>> First, we have a loop with Phi `0..2`.
>> 
>> for i in 0..2:  {body} i++
>> 
>> This is then peeled:
>> 
>> i = 0; {body}
>> for i in 1..2:  {body} i++
>> 
>> and 2x Unrolled
>> 
>> i = 0; {body}
>> for i in 1..2:
>>   Phi 1..2
>>   {body}
>>   i++
>>   Phi 2..2 -> collapses to constant
>>   {body}
>>   i++ -> collapses to constant int:3
>>   exit condition: i < limit   -> int:3 < limit_t
>> 
>> 
>> If the `CMoveI` cannot determine itself that the type is `int:<=3`, then the exit condition does not fold away, and we are left with a broken loop (`LoopNode` exists, but has no `Phi` it can reach from the `LoopEndNode`). This triggers a SIGSEGV if `-XX:+TraceLoopOpts` is enabled, or any other optimization that assumes the loop-phi to be present.
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   added stress and random flags/tags

Looks good to me.

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

Marked as reviewed by roland (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12092