- Revision
- 234060
- Author
- [email protected]
- Date
- 2018-07-20 12:16:29 -0700 (Fri, 20 Jul 2018)
Log Message
CompareEq should be using KnownOtherUse instead of OtherUse
https://bugs.webkit.org/show_bug.cgi?id=186814
<rdar://problem/39720030>
Reviewed by Filip Pizlo.
JSTests:
* stress/compare-eq-should-use-known-other-use.js: Added.
(bar):
(i.func):
Source/_javascript_Core:
CompareEq in fixup phase was doing this:
insertCheck(child, OtherUse)
setUseKind(child, OtherUse)
And in the DFG/FTL backend, it would not emit a check for OtherUse. This could
lead to edge verification crashing because a phase may optimize the check out
by removing the node. However, AI may not be privy to that optimization, and
AI may think the incoming value may not be Other. AI is expecting the DFG/FTL
backend to actually emit a check here, but it does not.
This exact pattern is why we have KnownXYZ use kinds. This patch introduces
KnownOtherUse and changes the above pattern to be:
insertCheck(child, OtherUse)
setUseKind(child, KnownOtherUse)
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGSafeToExecute.h:
(JSC::DFG::SafeToExecuteEdge::operator()):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::speculate):
* dfg/DFGUseKind.cpp:
(WTF::printInternal):
* dfg/DFGUseKind.h:
(JSC::DFG::typeFilterFor):
(JSC::DFG::shouldNotHaveTypeCheck):
(JSC::DFG::checkMayCrashIfInputIsEmpty):
* dfg/DFGWatchpointCollectionPhase.cpp:
(JSC::DFG::WatchpointCollectionPhase::handle):
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileCompareEq):
(JSC::FTL::DFG::LowerDFGToB3::speculate):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (234059 => 234060)
--- trunk/JSTests/ChangeLog 2018-07-20 18:39:40 UTC (rev 234059)
+++ trunk/JSTests/ChangeLog 2018-07-20 19:16:29 UTC (rev 234060)
@@ -1,3 +1,15 @@
+2018-07-20 Saam Barati <[email protected]>
+
+ CompareEq should be using KnownOtherUse instead of OtherUse
+ https://bugs.webkit.org/show_bug.cgi?id=186814
+ <rdar://problem/39720030>
+
+ Reviewed by Filip Pizlo.
+
+ * stress/compare-eq-should-use-known-other-use.js: Added.
+ (bar):
+ (i.func):
+
2018-07-20 David Fenton <[email protected]>
stress/spread-forward-varargs-stack-overflow.js is timing out in 32 bit JSC tests.
Added: trunk/JSTests/stress/compare-eq-should-use-known-other-use.js (0 => 234060)
--- trunk/JSTests/stress/compare-eq-should-use-known-other-use.js (rev 0)
+++ trunk/JSTests/stress/compare-eq-should-use-known-other-use.js 2018-07-20 19:16:29 UTC (rev 234060)
@@ -0,0 +1,29 @@
+function bar(testArgs) {
+ (() => {
+ try {
+ testArgs.func.call(1);
+ } catch (e) {
+ if (!testArgs.qux) {
+ return e == testArgs.qux;
+ }
+ }
+ })();
+}
+for (var i = 0; i < 100000; i++) {
+ [
+ {
+ func: ()=>{},
+ },
+ {
+ func: Int8Array.prototype.values,
+ foo: 0
+ },
+ {
+ func: Int8Array.prototype.values,
+ qux: 2
+ },
+ {
+ func: Int8Array.prototype.values,
+ },
+ ].forEach(bar);
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (234059 => 234060)
--- trunk/Source/_javascript_Core/ChangeLog 2018-07-20 18:39:40 UTC (rev 234059)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-07-20 19:16:29 UTC (rev 234060)
@@ -1,3 +1,45 @@
+2018-07-20 Saam Barati <[email protected]>
+
+ CompareEq should be using KnownOtherUse instead of OtherUse
+ https://bugs.webkit.org/show_bug.cgi?id=186814
+ <rdar://problem/39720030>
+
+ Reviewed by Filip Pizlo.
+
+ CompareEq in fixup phase was doing this:
+ insertCheck(child, OtherUse)
+ setUseKind(child, OtherUse)
+ And in the DFG/FTL backend, it would not emit a check for OtherUse. This could
+ lead to edge verification crashing because a phase may optimize the check out
+ by removing the node. However, AI may not be privy to that optimization, and
+ AI may think the incoming value may not be Other. AI is expecting the DFG/FTL
+ backend to actually emit a check here, but it does not.
+
+ This exact pattern is why we have KnownXYZ use kinds. This patch introduces
+ KnownOtherUse and changes the above pattern to be:
+ insertCheck(child, OtherUse)
+ setUseKind(child, KnownOtherUse)
+
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ * dfg/DFGSafeToExecute.h:
+ (JSC::DFG::SafeToExecuteEdge::operator()):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::speculate):
+ * dfg/DFGUseKind.cpp:
+ (WTF::printInternal):
+ * dfg/DFGUseKind.h:
+ (JSC::DFG::typeFilterFor):
+ (JSC::DFG::shouldNotHaveTypeCheck):
+ (JSC::DFG::checkMayCrashIfInputIsEmpty):
+ * dfg/DFGWatchpointCollectionPhase.cpp:
+ (JSC::DFG::WatchpointCollectionPhase::handle):
+ * ftl/FTLCapabilities.cpp:
+ (JSC::FTL::canCompile):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileCompareEq):
+ (JSC::FTL::DFG::LowerDFGToB3::speculate):
+
2018-07-20 Yusuke Suzuki <[email protected]>
[JSC] A bit performance improvement for Object.assign by cleaning up code
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (234059 => 234060)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2018-07-20 18:39:40 UTC (rev 234059)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2018-07-20 19:16:29 UTC (rev 234060)
@@ -587,21 +587,21 @@
// If either child can be proved to be Null or Undefined, comparing them is greatly simplified.
bool _oneArgumentIsUsedAsSpecOther_ = false;
if (node->child1()->isUndefinedOrNullConstant()) {
- fixEdge<OtherUse>(node->child1());
+ fixEdge<KnownOtherUse>(node->child1());
_oneArgumentIsUsedAsSpecOther_ = true;
} else if (node->child1()->shouldSpeculateOther()) {
m_insertionSet.insertNode(m_indexInBlock, SpecNone, Check, node->origin,
Edge(node->child1().node(), OtherUse));
- fixEdge<OtherUse>(node->child1());
+ fixEdge<KnownOtherUse>(node->child1());
_oneArgumentIsUsedAsSpecOther_ = true;
}
if (node->child2()->isUndefinedOrNullConstant()) {
- fixEdge<OtherUse>(node->child2());
+ fixEdge<KnownOtherUse>(node->child2());
_oneArgumentIsUsedAsSpecOther_ = true;
} else if (node->child2()->shouldSpeculateOther()) {
m_insertionSet.insertNode(m_indexInBlock, SpecNone, Check, node->origin,
Edge(node->child2().node(), OtherUse));
- fixEdge<OtherUse>(node->child2());
+ fixEdge<KnownOtherUse>(node->child2());
_oneArgumentIsUsedAsSpecOther_ = true;
}
if (oneArgumentIsUsedAsSpecOther) {
Modified: trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h (234059 => 234060)
--- trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h 2018-07-20 18:39:40 UTC (rev 234059)
+++ trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h 2018-07-20 19:16:29 UTC (rev 234060)
@@ -106,6 +106,11 @@
if (m_state.forNode(edge).m_type & ~(SpecHeapTop & ~SpecObject))
m_result = false;
return;
+
+ case KnownOtherUse:
+ if (m_state.forNode(edge).m_type & ~SpecOther)
+ m_result = false;
+ return;
case LastUseKind:
RELEASE_ASSERT_NOT_REACHED();
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (234059 => 234060)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2018-07-20 18:39:40 UTC (rev 234059)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2018-07-20 19:16:29 UTC (rev 234060)
@@ -10290,6 +10290,9 @@
case NotCellUse:
speculateNotCell(edge);
break;
+ case KnownOtherUse:
+ ASSERT(!needsTypeCheck(edge, SpecOther));
+ break;
case OtherUse:
speculateOther(edge);
break;
Modified: trunk/Source/_javascript_Core/dfg/DFGUseKind.cpp (234059 => 234060)
--- trunk/Source/_javascript_Core/dfg/DFGUseKind.cpp 2018-07-20 18:39:40 UTC (rev 234059)
+++ trunk/Source/_javascript_Core/dfg/DFGUseKind.cpp 2018-07-20 19:16:29 UTC (rev 234060)
@@ -154,6 +154,9 @@
case NotCellUse:
out.print("NotCell");
return;
+ case KnownOtherUse:
+ out.print("KnownOther");
+ return;
case OtherUse:
out.print("Other");
return;
Modified: trunk/Source/_javascript_Core/dfg/DFGUseKind.h (234059 => 234060)
--- trunk/Source/_javascript_Core/dfg/DFGUseKind.h 2018-07-20 18:39:40 UTC (rev 234059)
+++ trunk/Source/_javascript_Core/dfg/DFGUseKind.h 2018-07-20 19:16:29 UTC (rev 234060)
@@ -75,6 +75,7 @@
NotStringVarUse,
NotSymbolUse,
NotCellUse,
+ KnownOtherUse,
OtherUse,
MiscUse,
@@ -169,6 +170,7 @@
return ~SpecSymbol;
case NotCellUse:
return ~SpecCellCheck;
+ case KnownOtherUse:
case OtherUse:
return SpecOther;
case MiscUse:
@@ -188,6 +190,7 @@
case KnownStringUse:
case KnownPrimitiveUse:
case KnownBooleanUse:
+ case KnownOtherUse:
case Int52RepUse:
case DoubleRepUse:
return true;
@@ -313,6 +316,7 @@
case CellUse:
case KnownCellUse:
case CellOrOtherUse:
+ case KnownOtherUse:
case OtherUse:
case MiscUse:
case NotCellUse:
Modified: trunk/Source/_javascript_Core/dfg/DFGWatchpointCollectionPhase.cpp (234059 => 234060)
--- trunk/Source/_javascript_Core/dfg/DFGWatchpointCollectionPhase.cpp 2018-07-20 18:39:40 UTC (rev 234059)
+++ trunk/Source/_javascript_Core/dfg/DFGWatchpointCollectionPhase.cpp 2018-07-20 19:16:29 UTC (rev 234060)
@@ -81,7 +81,7 @@
if (m_node->isBinaryUseKind(ObjectUse)
|| (m_node->child1().useKind() == ObjectUse && m_node->child2().useKind() == ObjectOrOtherUse)
|| (m_node->child1().useKind() == ObjectOrOtherUse && m_node->child2().useKind() == ObjectUse)
- || (m_node->child1().useKind() == OtherUse || m_node->child2().useKind() == OtherUse))
+ || (m_node->child1().useKind() == KnownOtherUse || m_node->child2().useKind() == KnownOtherUse))
handleMasqueradesAsUndefined();
break;
Modified: trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp (234059 => 234060)
--- trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2018-07-20 18:39:40 UTC (rev 234059)
+++ trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2018-07-20 19:16:29 UTC (rev 234060)
@@ -447,6 +447,7 @@
case DerivedArrayUse:
case NotCellUse:
case OtherUse:
+ case KnownOtherUse:
case MiscUse:
case StringIdentUse:
case NotStringVarUse:
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (234059 => 234060)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-07-20 18:39:40 UTC (rev 234059)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-07-20 19:16:29 UTC (rev 234060)
@@ -6860,13 +6860,13 @@
return;
}
- if (m_node->child1().useKind() == OtherUse) {
+ if (m_node->child1().useKind() == KnownOtherUse) {
ASSERT(!m_interpreter.needsTypeCheck(m_node->child1(), SpecOther));
setBoolean(equalNullOrUndefined(m_node->child2(), AllCellsAreFalse, EqualNullOrUndefined, ManualOperandSpeculation));
return;
}
- if (m_node->child2().useKind() == OtherUse) {
+ if (m_node->child2().useKind() == KnownOtherUse) {
ASSERT(!m_interpreter.needsTypeCheck(m_node->child2(), SpecOther));
setBoolean(equalNullOrUndefined(m_node->child1(), AllCellsAreFalse, EqualNullOrUndefined, ManualOperandSpeculation));
return;
@@ -14839,6 +14839,7 @@
case KnownInt32Use:
case KnownStringUse:
case KnownPrimitiveUse:
+ case KnownOtherUse:
case DoubleRepUse:
case Int52RepUse:
ASSERT(!m_interpreter.needsTypeCheck(edge));
