The main JSC JIT has optimizations to speed up GetById and GetByVal for common cases. The DFG speculative JIT has some of these optimizations as well. But the non-speculative JIT always calls out to C, potentially resulting in performance regressions on code that fails speculation.
Created attachment 98226 [details] the patch
Attachment 98226 [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/ChangeLog:6: Line contains tab character. [whitespace/tab] [5] Source/JavaScriptCore/dfg/DFGJITCodeGenerator.cpp:328: Place brace on its own line for function definitions. [whitespace/braces] [4] Source/JavaScriptCore/dfg/DFGNonSpeculativeJIT.cpp:502: Missing spaces around = [whitespace/operators] [4] Source/JavaScriptCore/dfg/DFGNonSpeculativeJIT.cpp:503: Missing spaces around != [whitespace/operators] [3] Total errors found: 4 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 98230 [details] the patch (fixed style)
Attachment 98230 [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/ChangeLog:6: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 98231 [details] the patch (more style fixes)
Comment on attachment 98231 [details] the patch (more style fixes) View in context: https://bugs.webkit.org/attachment.cgi?id=98231&action=review Please fix comments above, as discussed! > Source/JavaScriptCore/dfg/DFGNonSpeculativeJIT.cpp:502 > + NodeIndex alias = node.child3; The alias optimization is unsafe on the non-speculative path. This assumes that between subsequent GetByVals to the same location, the value / array has not changed. We ensure this is true on the speculative path by restricting generation of operations that would trigger side-effects, but do not do so on the non-speculative path. > Source/JavaScriptCore/dfg/DFGNonSpeculativeJIT.cpp:586 > + cachedGetById(baseGPR, resultGPR, node.identifierNumber()); The code generation for this method is going to generate two calls, please refactor to make one call.
Created attachment 98280 [details] the patch (fixed review comments)
Attachment 98280 [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/DFGJITCodeGenerator.cpp:335: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 98300 [details] the patch (fixed review, fixed style)
Comment on attachment 98300 [details] the patch (fixed review, fixed style) Hey Filip, sorry to be picky but Darin will be disappointed in me if I'm not. :-) WebKit coding style calls for full names, bar cases of very commonly idiomatic abbreviations. really 'slowPathTarg' has to be 'slowPathTarget'. Sorry to bounce your patch for two extra letter, but we should do this right.
Created attachment 98411 [details] the patch (fix identifier name)
Comment on attachment 98411 [details] the patch (fix identifier name) Looks great Filip!
Comment on attachment 98411 [details] the patch (fix identifier name) Clearing flags on attachment: 98411 Committed r89643: <http://trac.webkit.org/changeset/89643>
All reviewed patches have been landed. Closing bug.