Bug 158206 – [jsc][mips] implement absDouble()

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Guillaume Emont]

The absDouble() method is not implemented in MacroAssemblerMIPS. It is now used in Math.pow() since r200208, so we need it.

[Guillaume Emont]

I have a patch that seems to work and will upload it after running some tests.

[Guillaume Emont]

(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]

Created attachment 280284 [details]
Patch

Patch fixing the issue.

[Julien Brianceau]

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]

(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]

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]

(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]

ping reviewers

[WebKit Commit Bot]

Comment on attachment 280338 [details]
Patch

Clearing flags on attachment: 280338

Committed r201716: <http://trac.webkit.org/changeset/201716>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.