Bug 44233 – Enable truncated floating point feature on ARM

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

[Gabor Loki]

We can enable truncated floating point fast cases to speed up several arithmetic tests.

[Gabor Loki]

Created attachment 64816 [details]
Enable truncated floating point feature on ARM

SunSpider: 1.011x as fast
V8: 1.003x as fast
WindScorpion: 1.003x as fast

[WebKit Review Bot]

Attachment 64816 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
JavaScriptCore/assembler/ARMAssembler.h:375:  cmn_r is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/assembler/ARMAssembler.h:587:  vcvtr_s32_f64_r is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 2 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gabor Loki]

Committed revision 65993.

[Yong Li]

(In reply to comment #3)
> Committed revision 65993.

This solution doesn't work for this test case:

var largeNeg=-2715228072;
alert(largeNeg >>> 5); // wrong when using ARM assembler.

Also see https://bugs.webkit.org/show_bug.cgi?id=67486

Should we revert this commit?

[Gavin Barraclough]

We should fix or revert.  Do you have time to look into this Gabor, or would you prefer we just roll out for now?

We should also add the test case to a layout test, if it is not already covered.

[Yong Li]

(In reply to comment #5)
> We should fix or revert.  Do you have time to look into this Gabor, or would you prefer we just roll out for now?
> 
> We should also add the test case to a layout test, if it is not already covered.

I attached a patch to https://bugs.webkit.org/show_bug.cgi?id=67486, which contains a test case. Should I move that patch to here and close 67486?

[Gavin Barraclough]

Hey Yong Li, thanks for the test case, and using a separate bug to track the roll out seems fine to me (this is what the sheriff bot does).

Rolled out by bug #67486

[Eric Seidel (no email)]

Attachment 64816 [details] was posted by a committer and has review+, assigning to Gabor Loki for commit.

[Gavin Barraclough]

This change was rolled out, looks like this is fixed by bug #67486 , and this change is now dead.  Closing as invalid.  If this is likely to be resurrected then apologies, and please feel free to reopen this bug.