Bug 149616 - Introduce SymbolUse optimization into CompareEq and CompareStrictEq
Summary: Introduce SymbolUse optimization into CompareEq and CompareStrictEq
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-29 00:46 PDT by Yusuke Suzuki
Modified: 2015-10-01 10:21 PDT (History)
7 users (show)

See Also:


Attachments
Patch (13.57 KB, patch)
2015-09-29 00:48 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (14.49 KB, patch)
2015-09-29 01:21 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (14.49 KB, patch)
2015-09-29 03:33 PDT, Yusuke Suzuki
saam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2015-09-29 00:46:07 PDT
Introduce SymbolUse optimization into CompareEq and CompareStrictEq
Comment 1 Yusuke Suzuki 2015-09-29 00:48:15 PDT
Created attachment 262049 [details]
Patch

WIP
Comment 2 WebKit Commit Bot 2015-09-29 00:51:12 PDT
Attachment 262049 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1239:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1300:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Yusuke Suzuki 2015-09-29 01:21:55 PDT
Created attachment 262051 [details]
Patch

WIP
Comment 4 WebKit Commit Bot 2015-09-29 01:25:19 PDT
Attachment 262051 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1239:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1300:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Yusuke Suzuki 2015-09-29 03:33:06 PDT
Created attachment 262058 [details]
Patch
Comment 6 WebKit Commit Bot 2015-09-29 03:35:29 PDT
Attachment 262058 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1239:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1300:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Yusuke Suzuki 2015-09-29 03:37:49 PDT
And next: https://bugs.webkit.org/show_bug.cgi?id=149622
Comment 8 Saam Barati 2015-09-29 13:23:30 PDT
Comment on attachment 262058 [details]
Patch

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

r=me with comment:

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3984
> +    if (node->isBinaryUseKind(SymbolUse)) {

Can you include some tests that include testing of peephole optimization?
Both for strict-equality and sloppy-equality.
Comment 9 Yusuke Suzuki 2015-10-01 09:52:36 PDT
Comment on attachment 262058 [details]
Patch

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

Thank you for your review :)

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3984
>> +    if (node->isBinaryUseKind(SymbolUse)) {
> 
> Can you include some tests that include testing of peephole optimization?
> Both for strict-equality and sloppy-equality.

After investigating the current implementation, I've found that compilePeepHoleSymbolEquality, compilePeepHoleObjectStrictEquality, compilePeepHoleBooleanBranch etc. is never executed now (at least, under the current JSC tests)
We have op_jless operation, but don't have op_jeq operation. So between op_eq and op_jtrue (or op_jfalse), we have MovHint to store the result of op_eq to the register. As a result, detectPeepHoleBranch() never detects the peephole.

compilePeepHoleDoubleBranch is currently called when we use op_jless, op_jgreater etc. But it is also not called when using `==`, and `===` ops.

For now, I added FIXME about this here. And open the bug for that.
https://bugs.webkit.org/show_bug.cgi?id=149713
Comment 10 Yusuke Suzuki 2015-10-01 10:21:19 PDT
Committed r190414: <http://trac.webkit.org/changeset/190414>