Bug 149616 – Introduce SymbolUse optimization into CompareEq and CompareStrictEq

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

[Yusuke Suzuki]

Introduce SymbolUse optimization into CompareEq and CompareStrictEq

[Yusuke Suzuki]

Created attachment 262049 [details]
Patch

WIP

[WebKit Commit Bot]

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.

[Yusuke Suzuki]

Created attachment 262051 [details]
Patch

WIP

[WebKit Commit Bot]

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.

[Yusuke Suzuki]

Created attachment 262058 [details]
Patch

[WebKit Commit Bot]

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.

[Yusuke Suzuki]

And next: https://bugs.webkit.org/show_bug.cgi?id=149622

[Saam Barati]

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.

[Yusuke Suzuki]

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

[Yusuke Suzuki]

Committed r190414: <http://trac.webkit.org/changeset/190414>