[9] RFR(S): 8160425: Vectorization with signalling NaN returns wrong result

[Tobias Hartmann]

Thanks, Vladimir!

Best regards,
Tobias

On 28.06.2016 18:04, Vladimir Kozlov wrote:
> Looks good.
> 
> thanks,
> Vladimir
> 
> On 6/28/16 6:29 AM, Tobias Hartmann wrote:
>> Hi Vladimir,
>>
>> thanks for the review!
>>
>> On 28.06.2016 15:20, Vladimir Ivanov wrote:
>>> test/compiler/vectorization/TestNaNVector.java:
>>>
>>>   56     public void vectorizeNaNDP() {
>>> ...
>>>   59         // should look like this '0xffa0ffa0ffa0ffa0' and is read from the constant
>>> ...
>>>   63         // as '0xffa0ffa0ffa0ffa0' which leads to an incorrect result.
>>>
>>> The comment in the test is misleading.
>>
>> Right, I first used a different constant and forgot to update the comment. The description in the RFR is correct.
>>
>> Also used the wrong bug id for the first webrev. Here's the new one:
>> http://cr.openjdk.java.net/~thartmann/8160425/webrev.01/
>>
>> Thanks,
>> Tobias
>>
>>> Otherwise, looks good.
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> On 6/28/16 2:47 PM, Tobias Hartmann wrote:
>>>> Hi,
>>>>
>>>> please review the following patch:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8160425
>>>> http://cr.openjdk.java.net/~thartmann/8160425/webrev.00/
>>>>
>>>> We vectorize the following loop:
>>>>
>>>> for (int i = 0; i < 1024; ++i) {
>>>>   array[i] = (char)0xfff7;
>>>> }
>>>>
>>>> The array store is replaced by a 64-bit vector store to four subsequent array elements. The vector should look like this '0xfff7fff7fff7fff7' and is read from the constant table. However, in floating point arithmetic, this is a signalling NaN which is converted to a quiet NaN when processed by the x87 FPU. After the signalling bit is set, the vector ends up in the constant table as '0xfffffff7fff7fff7' which leads to an incorrect result:
>>>>
>>>> [Constants]
>>>>   0xee376ec0 (offset:    0): 0xfff7fff7   0xfffffff7fff7fff7
>>>>   0xee376ec4 (offset:    4): 0xfffffff7
>>>>
>>>> The problem is that the constant vector is passed around as a double in the C code. On x86 32-bit, floating point values are returned via the FPU stack and since the value is a signalling NaN, it's converted to a quiet NaN by the FPU instructions. In this case, 'replicate8_imm' returns a double value which is passed to the caller on the FPU stack. Because the value is a signalling NaN, the 'fldl' instruction sets the most-significant fraction bit to 1 (see Chapter 4.8.3.4 of the Intel manual [1] or [2] for a similar problem).
>>>>
>>>> This code was introduced by JDK-7119644 [3]. I think we should not use doubles/floats in the C code but ints/longs because those constants should not be treated as floating point values. For consistency, I also changed this on SPARC although I'm not aware of any problems there. With the fix, the constant table contains the correct value:
>>>>
>>>> [Constants]
>>>>   0xee4154c0 (offset:    0): 0xfff7fff7   0xfff7fff7fff7fff7
>>>>   0xee4154c4 (offset:    4): 0xfff7fff7
>>>>
>>>> Tested with failing testcase, JPRT and RBT (running).
>>>>
>>>> Thanks,
>>>> Tobias
>>>>
>>>> [1] http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-manual-325462.pdf
>>>> [2] http://stackoverflow.com/questions/22816095/signalling-nan-was-corrupted-when-returning-from-x86-function-flds-fstps-of-x87
>>>> [3] https://bugs.openjdk.java.net/browse/JDK-7119644
>>>>