Bug 83666 – op_is_foo should be optimized

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

[Filip Pizlo]

Currently the op_is_foo opcodes result in C function calls, and the DFG doesn't handle them.

These opcodes should have fast inline assembly code and the DFG should handle them.

[Filip Pizlo]

Created attachment 136632 [details]
work in progress

[Early Warning System Bot]

Comment on attachment 136632 [details]
work in progress

Attachment 136632 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12380765

[Filip Pizlo]

Created attachment 136636 [details]
more

but still not done

[WebKit Review Bot]

Attachment 136636 [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/MacroAssemblerX86_64.h:422:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 19 files


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

[Early Warning System Bot]

Comment on attachment 136636 [details]
more

Attachment 136636 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12382707

[Early Warning System Bot]

Comment on attachment 136636 [details]
more

Attachment 136636 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12384672

[Build Bot]

Comment on attachment 136636 [details]
more

Attachment 136636 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12379912

[Filip Pizlo]

Created attachment 136757 [details]
ready for review but still needs a bit more love

[WebKit Review Bot]

Attachment 136757 [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/dfg/DFGOperations.h:88:  DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:422:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 24 files


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

[Build Bot]

Comment on attachment 136757 [details]
ready for review but still needs a bit more love

Attachment 136757 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12389138

[Early Warning System Bot]

Comment on attachment 136757 [details]
ready for review but still needs a bit more love

Attachment 136757 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12389152

[Early Warning System Bot]

Comment on attachment 136757 [details]
ready for review but still needs a bit more love

Attachment 136757 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12393019

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/113930

[Filip Pizlo]

I should note that I landed after fixing 32-bit and ARM bugs.  I will watch the bots to see that my work in this department was indeed fruitful and not completely destructive.

[Csaba Osztrogonác]

Reopen, because it broke the Qt-ARM build:

../../../../Source/JavaScriptCore/jit/JITOpcodes32_64.cpp: In member function 'void JSC::JIT::emit_op_is_string(JSC::Instruction*)':
../../../../Source/JavaScriptCore/jit/JITOpcodes32_64.cpp:683:101: error: 'compare8' was not declared in this scope

It seems compare8 is still missing from MacroAssemblerARM.h

[Filip Pizlo]

(In reply to comment #15)
> Reopen, because it broke the Qt-ARM build:
> 
> ../../../../Source/JavaScriptCore/jit/JITOpcodes32_64.cpp: In member function 'void JSC::JIT::emit_op_is_string(JSC::Instruction*)':
> ../../../../Source/JavaScriptCore/jit/JITOpcodes32_64.cpp:683:101: error: 'compare8' was not declared in this scope
> 
> It seems compare8 is still missing from MacroAssemblerARM.h

Weird, I would have expected http://trac.webkit.org/changeset/113934 to fix it.

[Csaba Osztrogonác]

Build fixed in http://trac.webkit.org/changeset/113955