Bug 83666

Summary:

op_is_foo should be optimized

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

loki, ossy, webkit.review.bot, zherczeg

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
work in progress	webkit-ews: commit-queue-
more	webkit-ews: commit-queue-
ready for review but still needs a bit more love	barraclough: review+, buildbot: commit-queue-

Filip Pizlo

Reported 2012-04-10 23:32:50 PDT

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.

Attachments
work in progress (15.13 KB, patch) 2012-04-10 23:35 PDT, Filip Pizlo	webkit-ews: commit-queue-	Details Formatted Diff Diff
more (23.18 KB, patch) 2012-04-11 00:01 PDT, Filip Pizlo	webkit-ews: commit-queue-	Details Formatted Diff Diff
ready for review but still needs a bit more love (44.54 KB, patch) 2012-04-11 14:36 PDT, Filip Pizlo	barraclough: review+ buildbot: commit-queue-	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2012-04-10 23:35:51 PDT

Created attachment 136632 [details] work in progress

Early Warning System Bot

Comment 2 2012-04-10 23:58:16 PDT

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

Comment 3 2012-04-11 00:01:12 PDT

Created attachment 136636 [details] more but still not done

WebKit Review Bot

Comment 4 2012-04-11 00:03:40 PDT

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 5 2012-04-11 00:07:43 PDT

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 6 2012-04-11 00:14:41 PDT

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 7 2012-04-11 00:17:52 PDT

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

Filip Pizlo

Comment 8 2012-04-11 14:36:31 PDT

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

WebKit Review Bot

Comment 9 2012-04-11 14:39:34 PDT

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 10 2012-04-11 14:52:52 PDT

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 11 2012-04-11 15:27:47 PDT

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 12 2012-04-11 15:40:15 PDT

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

Comment 13 2012-04-11 18:05:30 PDT

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

Filip Pizlo

Comment 14 2012-04-11 18:06:05 PDT

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

Comment 15 2012-04-11 23:08:48 PDT

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

Comment 16 2012-04-11 23:23:17 PDT

(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

Comment 17 2012-04-12 07:19:22 PDT

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

Note You need to log in before you can comment on or make changes to this bug.