Changeset 199639 in webkit

Timestamp:

Apr 16, 2016, 9:55:02 PM (10 years ago)

Author:

commit-queue@webkit.org

Message:

[JSC] DFG should support relational comparisons of Number and Other
​https://bugs.webkit.org/show_bug.cgi?id=156669

Patch by Benjamin Poulain <​bpoulain@webkit.org> on 2016-04-16
Reviewed by Darin Adler.

In Sunspider/3d-raytrace, DFG falls back to JSValue in some important
relational compare because profiling sees "undefined" from time to time.

This case is fairly common outside Sunspider too because of out-of-bounds array access.
Unfortunately for us, our fallback for compare is really inefficient.

Fortunately, relational comparison with null/undefined/true/false are trival.
We can just convert both side to Double. That's what this patch adds.

I also extended constant folding for those cases because I noticed
a bunch of "undefined" constant going through DoubleRep at runtime.

• dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

• dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode):

• tests/stress/compare-number-and-other.js: Added.

(opaqueSideEffect):
(let.operator.of.operators.eval.testPolymorphic):
(let.operator.of.operators.let.left.of.typeCases.let.right.of.typeCases.eval.testMonomorphic):
(let.operator.of.operators.let.left.of.typeCases.let.right.of.typeCases.testMonomorphicLeftConstant):
(let.operator.of.operators.let.left.of.typeCases.let.right.of.typeCases.testMonomorphicRightConstant):
(let.operator.of.operators.let.left.of.typeCases.let.right.of.typeCases.i.testPolymorphic):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGAbstractInterpreterInlines.h (modified) (1 diff)
• dfg/DFGFixupPhase.cpp (modified) (1 diff)
• tests/stress/compare-number-and-other.js (added)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-04-16  Benjamin Poulain  <bpoulain@webkit.org>
+
+        [JSC] DFG should support relational comparisons of Number and Other
+        https://bugs.webkit.org/show_bug.cgi?id=156669
+
+        Reviewed by Darin Adler.
+
+        In Sunspider/3d-raytrace, DFG falls back to JSValue in some important
+        relational compare because profiling sees "undefined" from time to time.
+
+        This case is fairly common outside Sunspider too because of out-of-bounds array access.
+        Unfortunately for us, our fallback for compare is really inefficient.
+
+        Fortunately, relational comparison with null/undefined/true/false are trival.
+        We can just convert both side to Double. That's what this patch adds.
+
+        I also extended constant folding for those cases because I noticed
+        a bunch of "undefined" constant going through DoubleRep at runtime.
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * tests/stress/compare-number-and-other.js: Added.
+        (opaqueSideEffect):
+        (let.operator.of.operators.eval.testPolymorphic):
+        (let.operator.of.operators.let.left.of.typeCases.let.right.of.typeCases.eval.testMonomorphic):
+        (let.operator.of.operators.let.left.of.typeCases.let.right.of.typeCases.testMonomorphicLeftConstant):
+        (let.operator.of.operators.let.left.of.typeCases.let.right.of.typeCases.testMonomorphicRightConstant):
+        (let.operator.of.operators.let.left.of.typeCases.let.right.of.typeCases.i.testPolymorphic):
+
 2016-04-16  Benjamin Poulain  <bpoulain@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h

@@ -393 +393 @@
     case DoubleRep: {
         JSValue child = forNode(node->child1()).value();
-        if (child && child.isNumber()) {
-            setConstant(node, jsDoubleNumber(child.asNumber()));
-            break;
+        if (child) {
+            if (child.isNumber()) {
+                setConstant(node, jsDoubleNumber(child.asNumber()));
+                break;
+            }
+            if (child.isUndefined()) {
+                setConstant(node, jsDoubleNumber(PNaN));
+                break;
+            }
+            if (child.isNull() || child.isFalse()) {
+                setConstant(node, jsDoubleNumber(0));
+                break;
+            }
+            if (child.isTrue()) {
+                setConstant(node, jsDoubleNumber(1));
+                break;
+            }
         }
 

trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

@@ -454 +454 @@
                 fixDoubleOrBooleanEdge(node->child1());
                 fixDoubleOrBooleanEdge(node->child2());
+            }
+            if (node->op() != CompareEq
+                && node->child1()->shouldSpeculateNotCell()
+                && node->child2()->shouldSpeculateNotCell()) {
+                if (node->child1()->shouldSpeculateNumberOrBoolean())
+                    fixDoubleOrBooleanEdge(node->child1());
+                else
+                    fixEdge<DoubleRepUse>(node->child1());
+                if (node->child2()->shouldSpeculateNumberOrBoolean())
+                    fixDoubleOrBooleanEdge(node->child2());
+                else
+                    fixEdge<DoubleRepUse>(node->child2());
                 node->clearFlags(NodeMustGenerate);
                 break;