[14] RFR(S): 8230062: assert(i == p->size()-1) failed: must be last element of the pack

[Christian Hagedorn]

Hi Tobias

Thank you for your review and your help/pointers along the way! It was a 
great learning experience.

Best regards,
Christian

On 25.09.19 08:18, Tobias Hartmann wrote:
> Hi Christian,
> 
> looks good to me!
> 
> Great work - knowing how much you suffered to get familiar with the various C2 optimizations that
> are involved and especially to extract this simple regression test.
> 
> Best regards,
> Tobias
> 
> On 24.09.19 16:13, Christian Hagedorn wrote:
>> Hi
>>
>> Please review the following patch:
>> https://bugs.openjdk.java.net/browse/JDK-8230062
>> http://cr.openjdk.java.net/~chagedorn/8230062/webrev.00/
>>
>> The assertion is too strong. There are cases where a reduction chain is put into a superword pack
>> but an intermediate result is used outside of the loop (see test case and [1]). This triggers the
>> assertion failure. Moreover, it would be incorrect to 'continue' in the loop on line 2054 (=
>> allowing a replacement with a vector instruction) and not exclude this pack.
>>
>> The fix simply adds the assertion check as a condition to the continue statement. If we encounter
>> such an intermediate result use outside of the loop then is_vector_use() returns false and we do not
>> use a vector instruction (i.e. return false on line 2056).
>>
>> Thank you!
>>
>> Best regards,
>> Christian
>>
>>
>> [1] https://bugs.openjdk.java.net/secure/attachment/84959/IntermediateResultUse.png