Bug 158206

Summary: [jsc][mips] implement absDouble()
Product: WebKit Reporter: Guillaume Emont <guijemont>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, commit-queue, guijemont, jbriance, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Other   
OS: Linux   
Attachments:
Description Flags
Patch
none
Patch none

Guillaume Emont
Reported 2016-05-30 08:18:55 PDT
The absDouble() method is not implemented in MacroAssemblerMIPS. It is now used in Math.pow() since r200208, so we need it.
Attachments
Patch (2.26 KB, patch)
2016-06-01 17:33 PDT, Guillaume Emont 		no flags
DetailsFormatted DiffDiff
Patch (2.66 KB, patch)
2016-06-02 10:47 PDT, Guillaume Emont 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Guillaume Emont
Comment 1 2016-05-30 08:24:45 PDT
I have a patch that seems to work and will upload it after running some tests.
Guillaume Emont
Comment 2 2016-05-30 10:18:02 PDT
(In reply to comment #1) > I have a patch that seems to work and will upload it after running some > tests. Can't do that yet as after rebasing my patch I hit #158209.
Guillaume Emont
Comment 3 2016-06-01 17:33:57 PDT
Created attachment 280284 [details] Patch Patch fixing the issue.
Julien Brianceau
Comment 4 2016-06-02 05:45:54 PDT
It is not complete, please refer to my commit in QtWebKit: http://code.qt.io/cgit/qt/qtwebkit.git/commit/?id=b6ddb5fe5d3f2223d524e45bf5cdbdde0e5b241f Also, please put absd() implementation between sqrtd and movd in MacroAssemblerMIPS.h to keep the opcodes ordered.
Guillaume Emont
Comment 5 2016-06-02 10:41:23 PDT
(In reply to comment #4) > It is not complete, please refer to my commit in QtWebKit: > http://code.qt.io/cgit/qt/qtwebkit.git/commit/ > ?id=b6ddb5fe5d3f2223d524e45bf5cdbdde0e5b241f Indeed, I did not catch that supportsFloatingPointAbs(), or see that you had made that change in qtwebkit. I think we need to check for WTF_MIPS_ISA_AT_LEAST(2) as well, since abs.d is only available from mips 2 on. > > Also, please put absd() implementation between sqrtd and movd in > MacroAssemblerMIPS.h to keep the opcodes ordered. It looks overall not very well ordered and I tried to put it in alphabetical order, but I'm fine with doing that.
Guillaume Emont
Comment 6 2016-06-02 10:47:24 PDT
Created attachment 280338 [details] Patch New version addressing Julien's comments. It seems to pass regress and stress tests if combined with patch from #158209
Julien Brianceau
Comment 7 2016-06-02 14:40:24 PDT
(In reply to comment #5) > Indeed, I did not catch that supportsFloatingPointAbs(), or see that you had > made that change in qtwebkit. I think we need to check for > WTF_MIPS_ISA_AT_LEAST(2) as well, since abs.d is only available from mips 2 > on. Great, thanks. > It looks overall not very well ordered and I tried to put it in alphabetical > order, but I'm fine with doing that. Thanks, it's just to limit conflicts in case of cherry-pick. Looks good to me.
Guillaume Emont
Comment 8 2016-06-06 10:39:04 PDT
ping reviewers
WebKit Commit Bot
Comment 9 2016-06-06 11:07:54 PDT
Comment on attachment 280338 [details] Patch Clearing flags on attachment: 280338 Committed r201716: <http://trac.webkit.org/changeset/201716>
WebKit Commit Bot
Comment 10 2016-06-06 11:07:59 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.