REGRESSION(r148790) Made 2 tests fail on x86 32bit
https://bugs.webkit.org/show_bug.cgi?id=114913
Summary REGRESSION(r148790) Made 2 tests fail on x86 32bit
Allan Sandfeld Jensen
Reported 2013-04-21 05:49:49 PDT
The fast/canvas tests only confirmed failing on the Qt bots, while the two sputnik tests also fails on the GTK bot. fast/canvas/canvas-arc-360-winding.html [ Failure ] fast/canvas/canvas-fillPath-alpha-shadow.html [ Failure ] fast/canvas/canvas-fillPath-gradient-shadow.html [ Failure ] fast/canvas/canvas-fillPath-pattern-shadow.html [ Failure ] fast/canvas/canvas-strokePath-alpha-shadow.html [ Failure ] sputnik/Conformance/15_Native_Objects/15.8_Math/15.8.2/15.8.2.13_pow/S15.8.2.13_A24.html [ Failure ] sputnik/Conformance/15_Native_Objects/15.8_Math/15.8.2/15.8.2.8_exp/S15.8.2.8_A6.html [ Failure ]
Attachments
Patch (1.69 KB, patch)
2013-04-22 06:38 PDT, Allan Sandfeld Jensen
no flags
Patch (5.62 KB, patch)
2013-04-23 06:08 PDT, Allan Sandfeld Jensen
fpizlo: review+
Allan Sandfeld Jensen
Comment 1 2013-04-22 06:35:42 PDT
The sputnik failure is related to bug 88519. The problem here being libc always uses x87 but doesn't configure the accuracy. Though unlike 88519 the problem is that it has too little accuracy if only given 64bit, exp and tan seems to need 80bit accuracy in x87 to give the right results back.
Allan Sandfeld Jensen
Comment 2 2013-04-22 06:38:53 PDT
Allan Sandfeld Jensen
Comment 3 2013-04-22 07:15:14 PDT
The two sputnik tests works if resetX87Stack is implemented as "finit". Or if an the CPU status word is loaded directly with 0x03FF (80bit FPU precision). The odd thing is though. That 0x02FF which we set the FPU precision to, is default on most operating systems. So the libc implementation is awefully fragile if it requires 80bit of FPU precision to give back the correct 64bit value.
Zoltan Arvai
Comment 4 2013-04-23 01:08:23 PDT
Tests marked to failure in http://trac.webkit.org/changeset/148948.
Zoltan Arvai
Comment 5 2013-04-23 01:32:15 PDT
For Ossy's suggestion failure mark changed to skip in http://trac.webkit.org/changeset/148950. Please unskip it with proper fix.
Allan Sandfeld Jensen
Comment 6 2013-04-23 06:08:11 PDT
Created attachment 199223 [details] Patch Make the code more reliable by getting rid of the resteX87Stack instruction and instead free the two registers at every call pseudo instructions. Also unskip the tests now.
Allan Sandfeld Jensen
Comment 7 2013-04-26 08:02:57 PDT
Filip, any objections?
Allan Sandfeld Jensen
Comment 8 2013-05-13 09:59:34 PDT
Filip: Ping?
Allan Sandfeld Jensen
Comment 9 2013-08-14 04:12:30 PDT
I will have to apply this patch to the Qt snapshots of WebKit with or without review, since it fixes JS regressions.
Allan Sandfeld Jensen
Comment 10 2013-08-14 04:25:49 PDT
I strongly suspect this is also what makes the newly unskipped test in https://bugs.webkit.org/show_bug.cgi?id=119792 fail on 32bit x86.
Allan Sandfeld Jensen
Comment 11 2013-08-15 04:25:46 PDT
Thanks Filip :)
Allan Sandfeld Jensen
Comment 12 2013-08-15 04:27:50 PDT
Allan Sandfeld Jensen
Comment 13 2013-08-15 04:30:13 PDT
Fixed the 5 tests caused by not resetting FPU registers. There are still two failures left caused by libc calculating slightly differently in exp and pow when FPU precision is set to 64bit. We earlier set FPU precission to 64bit because two other sputnik tests tested that we did not have excessive precision.
Filip Pizlo
Comment 14 2013-08-15 09:02:41 PDT
(In reply to comment #13) > Fixed the 5 tests caused by not resetting FPU registers. > > There are still two failures left caused by libc calculating slightly differently in exp and pow when FPU precision is set to 64bit. We earlier set FPU precission to 64bit because two other sputnik tests tested that we did not have excessive precision. Hmmm. ECMAScript allows for all transcendental Math functions to just have reasonable precision; if Test262 is testing specific results then the test is wrong. ECMAScript doesn't make any specific precision requirements. So, it may not even be required to set FPU precision to 64bit, unless the failing tests are tests of basic arithmetic (+, -, *, /, etc), which is the only place where ECMAScript actually requires 64bit precision. For Math tests, we should happily allow the tests to fail on x87, and we should lobby the Test262 folks to ammend the test to allow for wider precision.
Allan Sandfeld Jensen
Comment 15 2013-08-15 09:25:53 PDT
(In reply to comment #14) > (In reply to comment #13) > > Fixed the 5 tests caused by not resetting FPU registers. > > > > There are still two failures left caused by libc calculating slightly differently in exp and pow when FPU precision is set to 64bit. We earlier set FPU precission to 64bit because two other sputnik tests tested that we did not have excessive precision. > > Hmmm. ECMAScript allows for all transcendental Math functions to just have reasonable precision; if Test262 is testing specific results then the test is wrong. ECMAScript doesn't make any specific precision requirements. > > So, it may not even be required to set FPU precision to 64bit, unless the failing tests are tests of basic arithmetic (+, -, *, /, etc), which is the only place where ECMAScript actually requires 64bit precision. For Math tests, we should happily allow the tests to fail on x87, and we should lobby the Test262 folks to ammend the test to allow for wider precision. Unfortunately the two other tests are testing that basic arithmetics lose precision see https://bugs.webkit.org/show_bug.cgi?id=112239#c55 and https://bugs.webkit.org/show_bug.cgi?id=112239#c57
Note You need to log in before you can comment on or make changes to this bug.