Bug 73722 – Improve float array support in the DFG JIT

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Oliver Hunt]

Improve float array support in the DFG JIT

[Oliver Hunt]

Created attachment 117714 [details]
Patch

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/assembler/X86Assembler.h:1444:  cvtsd2ss_rr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:1450:  cvtss2sd_rr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:1502:  movsd_rm is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:1508:  movss_rm is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:1520:  movsd_mr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:1526:  movss_mr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/ChangeLog:10:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/JavaScriptCore/ChangeLog:12:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 8 in 11 files


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

[Gavin Barraclough]

Comment on attachment 117714 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=117714&action=review

r+ with ARMv7 bug & xor change.

> Source/JavaScriptCore/dfg/DFGNode.h:957
> +#if CPU(X86) || CPU(X86_64)

This should be ported to ARMv7.  Please file a bugzilla for this.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1678
> +        MacroAssembler::Jump notNaN = m_jit.branchDouble(MacroAssembler::DoubleEqual, fpr, fpr);

Maybe this should be functionality of the macro assembler, conversion of NaN should probably not be undefined, conversion to 0 is likely most sensible.  (though I think truncateDoubleFoo methods are currently used also used where we expect the value to be an integer, so we might need to switch to having a couple of more explicitly named versions of this functionality).

Please make a not in the ARMv7 bug to check whether this behavior is necessary on ARM (what does NaN convert to on ARM)?

The xorPtr should probably just be m_jit.move(Imm32(0), gpr).  The macro assembler should be free to implement this however it wishes (and on x86, yes, you may want to make this an xor).

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1751
> +        notNaN.link(&m_jit);

Please verify the float conversion doesn't maintain the payload on x86, and please make a note in the ARMv7 bug to also test this on ARM.

[Oliver Hunt]

Committed r101886: <http://trac.webkit.org/changeset/101886>