Should not predict OtherObj for ToThis with primitive types under strict mode
https://bugs.webkit.org/show_bug.cgi?id=153544
Reviewed by Filip Pizlo.
Currently, ToThis predicates OtherObj for primitive values.
But it's not true in strict mode.
In strict mode, ToThis does nothing on primitive values.
In this patch, we
1. fix prediction. Handles primitive types in strict mode. And we also handles StringObject.
2. convert it to Identity if the argument should be predicted as primitive types.
This optimization is important to implement Primitive.prototype.methods[1].
Otherwise, we always got BadType OSR exits.
[1]: https://bugs.webkit.org/show_bug.cgi?id=143889
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGConstantFoldingPhase.cpp:
(JSC::DFG::ConstantFoldingPhase::foldConstants):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::fixupToThis):
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
* tests/stress/to-this-boolean.js: Added.
(Boolean.prototype.negate):
(Boolean.prototype.negate2):
* tests/stress/to-this-double.js: Added.
(Number.prototype.negate):
* tests/stress/to-this-int32.js: Added.
(Number.prototype.negate):
* tests/stress/to-this-int52.js: Added.
(Number.prototype.negate):
* tests/stress/to-this-number.js: Added.
(Number.prototype.negate):
* tests/stress/to-this-string.js: Added.
(String.prototype.prefix):
(String.prototype.first):
(String.prototype.second):
* tests/stress/to-this-symbol.js: Added.
(Symbol.prototype.identity):
(Symbol.prototype.identity2):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@195938 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 98b3c34..c439021 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,52 @@
+2016-01-31 Yusuke Suzuki <utatane.tea@gmail.com>
+
+ Should not predict OtherObj for ToThis with primitive types under strict mode
+ https://bugs.webkit.org/show_bug.cgi?id=153544
+
+ Reviewed by Filip Pizlo.
+
+ Currently, ToThis predicates OtherObj for primitive values.
+ But it's not true in strict mode.
+ In strict mode, ToThis does nothing on primitive values.
+
+ In this patch, we
+
+ 1. fix prediction. Handles primitive types in strict mode. And we also handles StringObject.
+ 2. convert it to Identity if the argument should be predicted as primitive types.
+
+ This optimization is important to implement Primitive.prototype.methods[1].
+ Otherwise, we always got BadType OSR exits.
+
+ [1]: https://bugs.webkit.org/show_bug.cgi?id=143889
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGConstantFoldingPhase.cpp:
+ (JSC::DFG::ConstantFoldingPhase::foldConstants):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ (JSC::DFG::FixupPhase::fixupToThis):
+ * dfg/DFGPredictionPropagationPhase.cpp:
+ (JSC::DFG::PredictionPropagationPhase::propagate):
+ * tests/stress/to-this-boolean.js: Added.
+ (Boolean.prototype.negate):
+ (Boolean.prototype.negate2):
+ * tests/stress/to-this-double.js: Added.
+ (Number.prototype.negate):
+ * tests/stress/to-this-int32.js: Added.
+ (Number.prototype.negate):
+ * tests/stress/to-this-int52.js: Added.
+ (Number.prototype.negate):
+ * tests/stress/to-this-number.js: Added.
+ (Number.prototype.negate):
+ * tests/stress/to-this-string.js: Added.
+ (String.prototype.prefix):
+ (String.prototype.first):
+ (String.prototype.second):
+ * tests/stress/to-this-symbol.js: Added.
+ (Symbol.prototype.identity):
+ (Symbol.prototype.identity2):
+
2016-01-31 Guillaume Emont <guijemont@igalia.com>
[mips] don't save to a callee saved register too early
diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
index 9eec63c..d7c1bff 100644
--- a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
+++ b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
@@ -1691,10 +1691,21 @@
case ToThis: {
AbstractValue& source = forNode(node->child1());
AbstractValue& destination = forNode(node);
-
- if (m_graph.executableFor(node->origin.semantic)->isStrictMode())
+
+ if (source.m_type == SpecStringObject) {
+ m_state.setFoundConstants(true);
+ destination = source;
+ break;
+ }
+
+ if (m_graph.executableFor(node->origin.semantic)->isStrictMode()) {
+ if (!(source.m_type & ~(SpecFullNumber | SpecBoolean | SpecString | SpecSymbol))) {
+ m_state.setFoundConstants(true);
+ destination = source;
+ break;
+ }
destination.makeHeapTop();
- else {
+ } else {
destination = source;
destination.merge(SpecObject);
}
diff --git a/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp b/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp
index 3a56e35..e5be8ef 100644
--- a/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp
@@ -552,7 +552,7 @@
changed = true;
break;
}
-
+
case Check: {
alreadyHandled = true;
m_interpreter.execute(indexInBlock);
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index c9d5abf..2f235ad 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -1003,30 +1003,7 @@
}
case ToThis: {
- ECMAMode ecmaMode = m_graph.executableFor(node->origin.semantic)->isStrictMode() ? StrictMode : NotStrictMode;
-
- if (node->child1()->shouldSpeculateOther()) {
- if (ecmaMode == StrictMode) {
- fixEdge<OtherUse>(node->child1());
- node->convertToIdentity();
- break;
- }
-
- m_insertionSet.insertNode(
- m_indexInBlock, SpecNone, Check, node->origin,
- Edge(node->child1().node(), OtherUse));
- observeUseKindOnNode<OtherUse>(node->child1().node());
- m_graph.convertToConstant(
- node, m_graph.globalThisObjectFor(node->origin.semantic));
- break;
- }
-
- if (isFinalObjectSpeculation(node->child1()->prediction())) {
- fixEdge<FinalObjectUse>(node->child1());
- node->convertToIdentity();
- break;
- }
-
+ fixupToThis(node);
break;
}
@@ -1592,6 +1569,85 @@
node->convertToIdentity();
}
}
+
+ void fixupToThis(Node* node)
+ {
+ ECMAMode ecmaMode = m_graph.executableFor(node->origin.semantic)->isStrictMode() ? StrictMode : NotStrictMode;
+
+ if (ecmaMode == StrictMode) {
+ if (node->child1()->shouldSpeculateBoolean()) {
+ fixEdge<BooleanUse>(node->child1());
+ node->convertToIdentity();
+ return;
+ }
+
+ if (node->child1()->shouldSpeculateInt32()) {
+ fixEdge<Int32Use>(node->child1());
+ node->convertToIdentity();
+ return;
+ }
+
+ if (enableInt52() && node->child1()->shouldSpeculateMachineInt()) {
+ fixEdge<Int52RepUse>(node->child1());
+ node->convertToIdentity();
+ node->setResult(NodeResultInt52);
+ return;
+ }
+
+ if (node->child1()->shouldSpeculateNumber()) {
+ fixEdge<DoubleRepUse>(node->child1());
+ node->convertToIdentity();
+ node->setResult(NodeResultDouble);
+ return;
+ }
+
+ if (node->child1()->shouldSpeculateSymbol()) {
+ fixEdge<SymbolUse>(node->child1());
+ node->convertToIdentity();
+ return;
+ }
+
+ if (node->child1()->shouldSpeculateStringIdent()) {
+ fixEdge<StringIdentUse>(node->child1());
+ node->convertToIdentity();
+ return;
+ }
+
+ if (node->child1()->shouldSpeculateString()) {
+ fixEdge<StringUse>(node->child1());
+ node->convertToIdentity();
+ return;
+ }
+ }
+
+ if (node->child1()->shouldSpeculateOther()) {
+ if (ecmaMode == StrictMode) {
+ fixEdge<OtherUse>(node->child1());
+ node->convertToIdentity();
+ return;
+ }
+
+ m_insertionSet.insertNode(
+ m_indexInBlock, SpecNone, Check, node->origin,
+ Edge(node->child1().node(), OtherUse));
+ observeUseKindOnNode<OtherUse>(node->child1().node());
+ m_graph.convertToConstant(
+ node, m_graph.globalThisObjectFor(node->origin.semantic));
+ return;
+ }
+
+ if (node->child1()->shouldSpeculateStringObject()) {
+ fixEdge<StringObjectUse>(node->child1());
+ node->convertToIdentity();
+ return;
+ }
+
+ if (isFinalObjectSpeculation(node->child1()->prediction())) {
+ fixEdge<FinalObjectUse>(node->child1());
+ node->convertToIdentity();
+ return;
+ }
+ }
void fixupToPrimitive(Node* node)
{
diff --git a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
index 57998cf..fa48248 100644
--- a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
@@ -489,11 +489,58 @@
}
case ToThis: {
+ // ToThis in methods for primitive types should speculate primitive types in strict mode.
+ ECMAMode ecmaMode = m_graph.executableFor(node->origin.semantic)->isStrictMode() ? StrictMode : NotStrictMode;
+ if (ecmaMode == StrictMode) {
+ if (node->child1()->shouldSpeculateBoolean()) {
+ changed |= mergePrediction(SpecBoolean);
+ break;
+ }
+
+ if (node->child1()->shouldSpeculateInt32()) {
+ changed |= mergePrediction(SpecInt32);
+ break;
+ }
+
+ if (enableInt52() && node->child1()->shouldSpeculateMachineInt()) {
+ changed |= mergePrediction(SpecMachineInt);
+ break;
+ }
+
+ if (node->child1()->shouldSpeculateNumber()) {
+ changed |= mergePrediction(SpecMachineInt);
+ break;
+ }
+
+ if (node->child1()->shouldSpeculateSymbol()) {
+ changed |= mergePrediction(SpecSymbol);
+ break;
+ }
+
+ if (node->child1()->shouldSpeculateStringIdent()) {
+ changed |= mergePrediction(SpecStringIdent);
+ break;
+ }
+
+ if (node->child1()->shouldSpeculateString()) {
+ changed |= mergePrediction(SpecString);
+ break;
+ }
+ } else {
+ if (node->child1()->shouldSpeculateString()) {
+ changed |= mergePrediction(SpecStringObject);
+ break;
+ }
+ }
+
SpeculatedType prediction = node->child1()->prediction();
if (prediction) {
if (prediction & ~SpecObject) {
- prediction &= SpecObject;
- prediction = mergeSpeculations(prediction, SpecObjectOther);
+ // Wrapper objects are created only in sloppy mode.
+ if (ecmaMode != StrictMode) {
+ prediction &= SpecObject;
+ prediction = mergeSpeculations(prediction, SpecObjectOther);
+ }
}
changed |= mergePrediction(prediction);
}
diff --git a/Source/JavaScriptCore/tests/stress/to-this-boolean.js b/Source/JavaScriptCore/tests/stress/to-this-boolean.js
new file mode 100644
index 0000000..e5b3b76
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/to-this-boolean.js
@@ -0,0 +1,22 @@
+Boolean.prototype.negate = function ()
+{
+ "use strict";
+ return !this;
+};
+noInline(Boolean.prototype.negate);
+
+for (var i = 0; i < 1e4; ++i)
+ (i % 4 === 0).negate();
+Boolean.prototype.negate.call(true);
+
+for (var i = 0; i < 1e4; ++i)
+ Boolean.prototype.negate.call(i);
+
+Boolean.prototype.negate2 = function ()
+{
+ "use strict";
+ return !this;
+};
+
+for (var i = 0; i < 1e4; ++i)
+ (true).negate2();
diff --git a/Source/JavaScriptCore/tests/stress/to-this-double.js b/Source/JavaScriptCore/tests/stress/to-this-double.js
new file mode 100644
index 0000000..34b1728
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/to-this-double.js
@@ -0,0 +1,8 @@
+Number.prototype.negate = function ()
+{
+ "use strict";
+ return -this;
+};
+
+for (var i = 1; i < 1e4; ++i)
+ (4.24242).negate();
diff --git a/Source/JavaScriptCore/tests/stress/to-this-int32.js b/Source/JavaScriptCore/tests/stress/to-this-int32.js
new file mode 100644
index 0000000..7cf0c2a
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/to-this-int32.js
@@ -0,0 +1,8 @@
+Number.prototype.negate = function ()
+{
+ "use strict";
+ return -this;
+};
+
+for (var i = 1; i < 1e4; ++i)
+ (0x424242).negate();
diff --git a/Source/JavaScriptCore/tests/stress/to-this-int52.js b/Source/JavaScriptCore/tests/stress/to-this-int52.js
new file mode 100644
index 0000000..145f7a9
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/to-this-int52.js
@@ -0,0 +1,8 @@
+Number.prototype.negate = function ()
+{
+ "use strict";
+ return -this;
+};
+
+for (var i = 1; i < 1e4; ++i)
+ (0xfffffff * 100000).negate();
diff --git a/Source/JavaScriptCore/tests/stress/to-this-number.js b/Source/JavaScriptCore/tests/stress/to-this-number.js
new file mode 100644
index 0000000..2f13a9f
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/to-this-number.js
@@ -0,0 +1,20 @@
+Number.prototype.negate = function ()
+{
+ "use strict";
+ return -this;
+};
+noInline(Number.prototype.negate);
+
+for (var i = 0; i < 1e4; ++i)
+ (i % 3 === 0 ? -i : i).negate();
+
+for (var i = 0; i < 1e4; ++i)
+ ((i % 3 === 0 ? -i : i) * 0.2).negate();
+
+for (var i = 0; i < 1e4; ++i)
+ ((i % 3 === 0 ? -i : i) * 1000000).negate();
+
+Number.prototype.negate.call(-20000);
+
+for (var i = 0; i < 1e4; ++i)
+ Number.prototype.negate.call(i % 2 === 0);
diff --git a/Source/JavaScriptCore/tests/stress/to-this-string.js b/Source/JavaScriptCore/tests/stress/to-this-string.js
new file mode 100644
index 0000000..0d4f9b1c
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/to-this-string.js
@@ -0,0 +1,27 @@
+String.prototype.prefix = function (string)
+{
+ "use strict";
+ // ToThis should be converted to Identity.
+ return string + this;
+};
+noInline(String.prototype.prefix);
+
+String.prototype.first = function (string)
+{
+ return this.second(string);
+};
+
+String.prototype.second = function (string)
+{
+ // Duplicate ToThis(in first) should be converted to Identity.
+ return this + string;
+};
+noInline(String.prototype.first);
+
+
+for (var i = 0; i < 1e4; ++i)
+ String(i).prefix("Hello");
+
+
+for (var i = 0; i < 1e4; ++i)
+ String(i).first("Hello");
diff --git a/Source/JavaScriptCore/tests/stress/to-this-symbol.js b/Source/JavaScriptCore/tests/stress/to-this-symbol.js
new file mode 100644
index 0000000..e4520ab
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/to-this-symbol.js
@@ -0,0 +1,18 @@
+Symbol.prototype.identity = function ()
+{
+ "use strict";
+ return this;
+};
+noInline(Symbol.prototype.identity);
+
+Symbol.prototype.identity2 = function ()
+{
+ "use strict";
+ return this;
+};
+
+for (var i = 1; i < 1e4; ++i)
+ Symbol.prototype.identity.call(Symbol(i));
+
+for (var i = 1; i < 1e4; ++i)
+ Symbol.prototype.identity2.call(Symbol(i));
