RFR: 8267399: C2: java/text/Normalizer/ConformanceTest.java test failed with assertion

[Roland Westrelin]

On Mon, 14 Jun 2021 18:31:56 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> This was found with Shenandoah but has nothing specific to Shenandoah.
>> 
>> For the inner loop of the test case:
>> 
>> - the loop limit, len, is known to be either 1 or 2. As a consequence
>>   the type of the iv phi is set to [0..2].
>> 
>> - range check elimination is applied which causes the pre/main/post
>>   loops to be added and the limit of the main loop to be adjusted. C2
>>   computes the limit of the main loop as TypeINT:INT.
>> 
>> - the main loop is peeled. So now the type of the iv phi is 2 and it
>>   constant folds. Because the main loop limit has type TypeInt::INT,
>>   the exit condition of the loop doesn't constant fold. But the loop
>>   no longer has the shape of a counted loop because the iv phi doesn't
>>   exist anymore.
>> 
>> The crash then occurs when some other loop opts is attempted and loop
>> strip mining verification code checks that the shape of the main loop
>> is that of a counted loop.
>> 
>> The fix I propose is to add a CastII at range elimination time that
>> captures the bounds of the limit before RC is applied. This way the
>> type of the limit is not lost and the main loop backbranch can be
>> properly eliminated in the chain of events above.
>
> src/hotspot/share/opto/loopTransform.cpp line 2858:
> 
>> 2856:   bool upward = cl->stride_con() > 0;
>> 2857:   main_limit = new CastIINode(main_limit, TypeInt::make(upward ? min_jint : orig_limit_t->_lo,
>> 2858:                                                         upward ? orig_limit_t->_hi : max_jint, Type::WidenMax));
> 
> First, need comment.
> Second, why you used min_jint and max_jint instead of main_limit->_lo and main_limit->_hi?

Thanks for the reviewing this.

> Second, why you used min_jint and max_jint instead of main_limit->_lo and main_limit->_hi?

Range check elimination changes the main loop's limit. For a loop going up, the new limit is less or equal to the initial limit. So it's less than main_limit->_hi. But it's not guaranteed to be greater or equal to main_limit->_lo. Say, main_limit is a constant, then main_limit->_lo = main_limit->_hi. The new limit can't be >= main_limit->_lo otherwise the new limit is main_limit->_lo = main_limit->_hi and the limit is unchanged.

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

PR: https://git.openjdk.java.net/jdk/pull/4388