Bug 324816 - Incorrect VEX implementation for xscvspdp/xvcvspdp regarding SNaN inputs
Summary: Incorrect VEX implementation for xscvspdp/xvcvspdp regarding SNaN inputs
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: vex (show other bugs)
Version: 3.9.0.SVN
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-11 23:06 UTC by Maynard Johnson
Modified: 2013-09-12 17:48 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Patch to fix this bug (238.74 KB, patch)
2013-09-11 23:07 UTC, Maynard Johnson
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Maynard Johnson 2013-09-11 23:06:56 UTC
The Power ISA 2.07 document includes a correction to the description for the behavior of the xscvspdp instruction, indicating that if the source argument is a SNaN, it is first changed to a QNaN before being converted from single-precision to double-precision. This updated information about the xscvspdp instruction exposed a bug in the VEX implementation for that instruction and also a bug in the testing for all instructions having special behavior for single-precision SNaN arguments. This patch fixes both the VEX bug and the testing bug.

VEX bug in xscvspdp implementation:
The current implementation of xscvspdp emulates the instruction by extracting the single-precision floating point from the vector register, storing it in single-prcision, and then loading the data just stored using the lfsx instruction. But the lfsx instruction does not change SNaN input arguments to QNaN inputs before conversion to double-precision, so this emulation is not sufficient for the xscvspdp instruction as described in the current documentation.  This patch fixes that issue by recognizing a SNaN input and changing it to a QNaN before performing the emulation using lfsx.

While fixing the bug in xscvspdp implementation, I also discovered that xvcvspdp had the same issue where SNaN inputs were not being handled correctly, so this patch also fixes its implementation, too

Testing bug:
In several ppc[64] test cases, an array of special double-precision floating point values is set up, and then all elements of that array are copied via assignment to a single-precision array ('float' type).  Assignment from a double to a float works fine for all cases, except for SNaN values. In the
case of a SNaN, the source is changed to a QNaN and then converted to single-precision.  So the end result was that our array of floats did not have an
actual SNaN value, and, therefore, any instructions that had special behavior for a single-precision SNaN input argument was never being properly tested.
This patch makes some functional changes in the following testcases:
   none/tests/ppc[32|64]/test_isa_2_06_part2.c
   none/tests/ppc[32|64]/test_isa_2_06_part3.c
   none/tests/ppc[32|64]/test_isa_2_07_part2.c

These changes impacted the associated *.stdout.exp files, so the patch also updates those files.

Additionally, there were several errors in testcase source comments that misidentified QNaN and SNaN bit patterns which this patch corrects.
Comment 1 Maynard Johnson 2013-09-11 23:07:35 UTC
Created attachment 82288 [details]
Patch to fix this bug
Comment 2 Julian Seward 2013-09-12 15:22:59 UTC
Carl, does this look OK to you?
Comment 3 Carl Love 2013-09-12 15:54:43 UTC
Julian, I am still working with Maynard on this issue. The initial patch had an issue.  Waiting for the patch to be updated.
Comment 4 Carl Love 2013-09-12 17:39:34 UTC
A little confusion on my part, the attached patch was the updated one.  My confusion.

Patch committed.   VEX commit r2760, Valgrind commit r13544