DFG and FTL should specialize for and support CompareStrictEq over Misc (i.e. boolean, undefined, or null)
https://bugs.webkit.org/show_bug.cgi?id=129563
Source/JavaScriptCore:
Reviewed by Geoffrey Garen.
Rolling this back in after fixing an assertion failure. speculateMisc() should have
said DFG_TYPE_CHECK instead of typeCheck.
This adds a specialization of CompareStrictEq over Misc. I noticed the need for this
when I saw that we didn't support CompareStrictEq(Untyped) in FTL but that the main
user of this was EarleyBoyer, and in that benchmark what it was really doing was
comparing undefined, null, and booleans to each other.
This also adds support for miscellaneous things that I needed to make my various test
cases work. This includes comparison over booleans and the various Throw-related node
types.
This also improves constant folding of CompareStrictEq and CompareEq.
Also found a bug where we were claiming that GetByVals on typed arrays are OutOfBounds
based on profiling, which caused some downstream badness. We don't actually support
compiling OutOfBounds GetByVals on typed arrays. The DFG would ignore the flag and just
emit a bounds check, but in the FTL path, the SSA lowering phase would assume that it
shouldn't factor out the bounds check since the access is not InBounds but then the
backend would ignore the flag and assume that the bounds check was already emitted.
This showed up on an existing test but I added a test for this explicitly to have more
certain coverage. The fix is to not mark something as OutOfBounds if the semantics are
that we'll have a bounds check anyway.
This is a 1% speed-up on Octane mostly because of raytrace, but also because of just
general progressions across the board. No speed-up yet on EarleyBoyer, since there is
still a lot more coverage work to be done there.
* bytecode/SpeculatedType.cpp:
(JSC::speculationToAbbreviatedString):
(JSC::leastUpperBoundOfStrictlyEquivalentSpeculations):
(JSC::valuesCouldBeEqual):
* bytecode/SpeculatedType.h:
(JSC::isMiscSpeculation):
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGArrayMode.cpp:
(JSC::DFG::ArrayMode::refine):
* dfg/DFGArrayMode.h:
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::attemptToMakeGetArrayLength):
* dfg/DFGNode.h:
(JSC::DFG::Node::shouldSpeculateMisc):
* dfg/DFGSafeToExecute.h:
(JSC::DFG::SafeToExecuteEdge::operator()):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileStrictEq):
(JSC::DFG::SpeculativeJIT::speculateMisc):
(JSC::DFG::SpeculativeJIT::speculate):
* dfg/DFGSpeculativeJIT.h:
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compileMiscStrictEq):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compileMiscStrictEq):
* dfg/DFGUseKind.cpp:
(WTF::printInternal):
* dfg/DFGUseKind.h:
(JSC::DFG::typeFilterFor):
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::compileNode):
(JSC::FTL::LowerDFGToLLVM::compileCompareEq):
(JSC::FTL::LowerDFGToLLVM::compileCompareStrictEq):
(JSC::FTL::LowerDFGToLLVM::compileThrow):
(JSC::FTL::LowerDFGToLLVM::isNotMisc):
(JSC::FTL::LowerDFGToLLVM::isMisc):
(JSC::FTL::LowerDFGToLLVM::speculate):
(JSC::FTL::LowerDFGToLLVM::speculateMisc):
* tests/stress/float32-array-out-of-bounds.js: Added.
* tests/stress/weird-equality-folding-cases.js: Added.
LayoutTests:
Reviewed by Geoffrey Garen.
* js/regress/fold-strict-eq-expected.txt: Added.
* js/regress/fold-strict-eq.html: Added.
* js/regress/misc-strict-eq-expected.txt: Added.
* js/regress/misc-strict-eq.html: Added.
* js/regress/script-tests/fold-strict-eq.js: Added.
(foo):
(test):
* js/regress/script-tests/misc-strict-eq.js: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@165099 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
index 32ed82f..dbf88ff 100644
--- a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
+++ b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
@@ -917,8 +917,7 @@
if (node->op() == CompareEqConstant || node->op() == CompareEq) {
SpeculatedType leftType = forNode(node->child1()).m_type;
SpeculatedType rightType = forNode(node->child2()).m_type;
- if ((isInt32Speculation(leftType) && isOtherSpeculation(rightType))
- || (isOtherSpeculation(leftType) && isInt32Speculation(rightType))) {
+ if (!valuesCouldBeEqual(leftType, rightType)) {
setConstant(node, jsBoolean(false));
break;
}
@@ -943,19 +942,28 @@
JSValue left = forNode(leftNode).value();
JSValue right = forNode(rightNode).value();
if (left && right) {
- if (left.isNumber() && right.isNumber()) {
- setConstant(node, jsBoolean(left.asNumber() == right.asNumber()));
- break;
- }
if (left.isString() && right.isString()) {
+ // We need this case because JSValue::strictEqual is otherwise too racy for
+ // string comparisons.
const StringImpl* a = asString(left)->tryGetValueImpl();
const StringImpl* b = asString(right)->tryGetValueImpl();
if (a && b) {
setConstant(node, jsBoolean(WTF::equal(a, b)));
break;
}
+ } else {
+ setConstant(node, jsBoolean(JSValue::strictEqual(0, left, right)));
+ break;
}
}
+
+ SpeculatedType leftLUB = leastUpperBoundOfStrictlyEquivalentSpeculations(forNode(leftNode).m_type);
+ SpeculatedType rightLUB = leastUpperBoundOfStrictlyEquivalentSpeculations(forNode(rightNode).m_type);
+ if (!(leftLUB & rightLUB)) {
+ setConstant(node, jsBoolean(false));
+ break;
+ }
+
forNode(node).setType(SpecBoolean);
node->setCanExit(true); // This is overly conservative.
break;
diff --git a/Source/JavaScriptCore/dfg/DFGArrayMode.cpp b/Source/JavaScriptCore/dfg/DFGArrayMode.cpp
index 1dd9f31..9a93980 100644
--- a/Source/JavaScriptCore/dfg/DFGArrayMode.cpp
+++ b/Source/JavaScriptCore/dfg/DFGArrayMode.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012, 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2012, 2013, 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -132,7 +132,7 @@
}
ArrayMode ArrayMode::refine(
- Graph& graph, CodeOrigin codeOrigin,
+ Graph& graph, Node* node,
SpeculatedType base, SpeculatedType index, SpeculatedType value,
NodeFlags flags) const
{
@@ -199,10 +199,18 @@
return withType(Array::Arguments);
ArrayMode result;
- if (graph.hasExitSite(codeOrigin, OutOfBounds) || !isInBounds())
- result = withSpeculation(Array::OutOfBounds);
- else
+ switch (node->op()) {
+ case PutByVal:
+ if (graph.hasExitSite(node->origin.semantic, OutOfBounds) || !isInBounds())
+ result = withSpeculation(Array::OutOfBounds);
+ else
+ result = withSpeculation(Array::InBounds);
+ break;
+
+ default:
result = withSpeculation(Array::InBounds);
+ break;
+ }
if (isInt8ArraySpeculation(base))
return result.withType(Array::Int8Array);
diff --git a/Source/JavaScriptCore/dfg/DFGArrayMode.h b/Source/JavaScriptCore/dfg/DFGArrayMode.h
index add9215..9c67edb 100644
--- a/Source/JavaScriptCore/dfg/DFGArrayMode.h
+++ b/Source/JavaScriptCore/dfg/DFGArrayMode.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012, 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2012, 2013, 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -220,7 +220,7 @@
return ArrayMode(type, arrayClass(), speculation(), conversion);
}
- ArrayMode refine(Graph&, CodeOrigin, SpeculatedType base, SpeculatedType index, SpeculatedType value = SpecNone, NodeFlags = 0) const;
+ ArrayMode refine(Graph&, Node*, SpeculatedType base, SpeculatedType index, SpeculatedType value = SpecNone, NodeFlags = 0) const;
bool alreadyChecked(Graph&, Node*, AbstractValue&) const;
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index b90c317..5c0aa42 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -432,6 +432,11 @@
fixEdge<ObjectUse>(node->child2());
break;
}
+ if (node->child1()->shouldSpeculateMisc() && node->child2()->shouldSpeculateMisc()) {
+ fixEdge<MiscUse>(node->child1());
+ fixEdge<MiscUse>(node->child2());
+ break;
+ }
break;
}
@@ -452,7 +457,7 @@
case GetByVal: {
node->setArrayMode(
node->arrayMode().refine(
- m_graph, node->origin.semantic,
+ m_graph, node,
node->child1()->prediction(),
node->child2()->prediction(),
SpecNone, node->flags()));
@@ -510,7 +515,7 @@
node->setArrayMode(
node->arrayMode().refine(
- m_graph, node->origin.semantic,
+ m_graph, node,
child1->prediction(),
child2->prediction(),
child3->prediction()));
@@ -596,7 +601,7 @@
// that would break things.
node->setArrayMode(
node->arrayMode().refine(
- m_graph, node->origin.semantic,
+ m_graph, node,
node->child1()->prediction() & SpecCell,
SpecInt32,
node->child2()->prediction()));
@@ -1750,7 +1755,7 @@
}
arrayMode = arrayMode.refine(
- m_graph, node->origin.semantic, node->child1()->prediction(), node->prediction());
+ m_graph, node, node->child1()->prediction(), node->prediction());
if (arrayMode.type() == Array::Generic) {
// Check if the input is something that we can't get array length for, but for which we
diff --git a/Source/JavaScriptCore/dfg/DFGNode.h b/Source/JavaScriptCore/dfg/DFGNode.h
index 690f1f5..b5b9590 100644
--- a/Source/JavaScriptCore/dfg/DFGNode.h
+++ b/Source/JavaScriptCore/dfg/DFGNode.h
@@ -1399,6 +1399,11 @@
{
return isBooleanSpeculation(prediction());
}
+
+ bool shouldSpeculateMisc()
+ {
+ return isMiscSpeculation(prediction());
+ }
bool shouldSpeculateStringIdent()
{
diff --git a/Source/JavaScriptCore/dfg/DFGSafeToExecute.h b/Source/JavaScriptCore/dfg/DFGSafeToExecute.h
index 1bc1fbc..06c0d40 100644
--- a/Source/JavaScriptCore/dfg/DFGSafeToExecute.h
+++ b/Source/JavaScriptCore/dfg/DFGSafeToExecute.h
@@ -59,6 +59,7 @@
case StringOrStringObjectUse:
case NotCellUse:
case OtherUse:
+ case MiscUse:
case MachineIntUse:
return;
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
index 417e760..2641fbb 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -3802,6 +3802,11 @@
return false;
}
+ case MiscUse: {
+ compileMiscStrictEq(node);
+ return false;
+ }
+
case UntypedUse: {
return nonSpeculativeStrictEq(node);
}
@@ -4830,6 +4835,31 @@
#endif
}
+void SpeculativeJIT::speculateMisc(Edge edge, JSValueRegs regs)
+{
+#if USE(JSVALUE64)
+ DFG_TYPE_CHECK(
+ regs, edge, SpecMisc,
+ m_jit.branch64(MacroAssembler::Above, regs.gpr(), MacroAssembler::TrustedImm64(TagBitTypeOther | TagBitBool | TagBitUndefined)));
+#else
+ DFG_TYPE_CHECK(
+ regs, edge, SpecMisc | SpecInt32,
+ m_jit.branch32(MacroAssembler::Equal, regs.tagGPR(), MacroAssembler::TrustedImm32(JSValue::Int32Tag)));
+ DFG_TYPE_CHECK(
+ regs, edge, SpecMisc,
+ m_jit.branch32(MacroAssembler::Below, regs.tagGPR(), MacroAssembler::TrustedImm32(JSValue::UndefinedTag)));
+#endif
+}
+
+void SpeculativeJIT::speculateMisc(Edge edge)
+{
+ if (!needsTypeCheck(edge, SpecMisc))
+ return;
+
+ JSValueOperand operand(this, edge, ManualOperandSpeculation);
+ speculateMisc(edge, operand.jsValueRegs());
+}
+
void SpeculativeJIT::speculate(Node*, Edge edge)
{
switch (edge.useKind()) {
@@ -4892,6 +4922,9 @@
case OtherUse:
speculateOther(edge);
break;
+ case MiscUse:
+ speculateMisc(edge);
+ break;
default:
RELEASE_ASSERT_NOT_REACHED();
break;
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
index 391d1ed..df4f96d 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
@@ -1985,6 +1985,7 @@
void compileStringEquality(Node*);
void compileStringIdentEquality(Node*);
void compileStringZeroLength(Node*);
+ void compileMiscStrictEq(Node*);
void emitObjectOrOtherBranch(Edge value, BasicBlock* taken, BasicBlock* notTaken);
void emitBranch(Node*);
@@ -2203,6 +2204,8 @@
void speculateStringOrStringObject(Edge);
void speculateNotCell(Edge);
void speculateOther(Edge);
+ void speculateMisc(Edge, JSValueRegs);
+ void speculateMisc(Edge);
void speculate(Node*, Edge);
JITCompiler::Jump jumpSlowForUnwantedArrayMode(GPRReg tempWithIndexingTypeReg, ArrayMode, IndexingType);
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
index 1cf356c..9e0097a 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
@@ -613,6 +613,22 @@
booleanResult(resultPayloadGPR, node, UseChildrenCalledExplicitly);
}
+void SpeculativeJIT::compileMiscStrictEq(Node* node)
+{
+ JSValueOperand op1(this, node->child1(), ManualOperandSpeculation);
+ JSValueOperand op2(this, node->child2(), ManualOperandSpeculation);
+ GPRTemporary result(this);
+
+ speculateMisc(node->child1(), op1.jsValueRegs());
+ speculateMisc(node->child2(), op2.jsValueRegs());
+
+ m_jit.move(TrustedImm32(0), result.gpr());
+ JITCompiler::Jump notEqual = m_jit.branch32(JITCompiler::NotEqual, op1.tagGPR(), op2.tagGPR());
+ m_jit.compare32(JITCompiler::Equal, op1.payloadGPR(), op2.payloadGPR(), result.gpr());
+ notEqual.link(&m_jit);
+ booleanResult(result.gpr(), node);
+}
+
void SpeculativeJIT::emitCall(Node* node)
{
if (node->op() != Call)
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
index 475b4db..c3c95db 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
@@ -648,6 +648,20 @@
jsValueResult(resultGPR, m_currentNode, DataFormatJSBoolean, UseChildrenCalledExplicitly);
}
+void SpeculativeJIT::compileMiscStrictEq(Node* node)
+{
+ JSValueOperand op1(this, node->child1(), ManualOperandSpeculation);
+ JSValueOperand op2(this, node->child2(), ManualOperandSpeculation);
+ GPRTemporary result(this);
+
+ speculateMisc(node->child1(), op1.jsValueRegs());
+ speculateMisc(node->child2(), op2.jsValueRegs());
+
+ m_jit.compare32(JITCompiler::Equal, op1.gpr(), op2.gpr(), result.gpr());
+ m_jit.or32(TrustedImm32(ValueFalse), result.gpr());
+ jsValueResult(result.gpr(), node, DataFormatJSBoolean);
+}
+
void SpeculativeJIT::emitCall(Node* node)
{
if (node->op() != Call)
diff --git a/Source/JavaScriptCore/dfg/DFGUseKind.cpp b/Source/JavaScriptCore/dfg/DFGUseKind.cpp
index 4e6df98..b7a5c6f 100644
--- a/Source/JavaScriptCore/dfg/DFGUseKind.cpp
+++ b/Source/JavaScriptCore/dfg/DFGUseKind.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -97,6 +97,9 @@
case OtherUse:
out.print("Other");
break;
+ case MiscUse:
+ out.print("Misc");
+ break;
default:
RELEASE_ASSERT_NOT_REACHED();
break;
diff --git a/Source/JavaScriptCore/dfg/DFGUseKind.h b/Source/JavaScriptCore/dfg/DFGUseKind.h
index dfa82ee..f66d143 100644
--- a/Source/JavaScriptCore/dfg/DFGUseKind.h
+++ b/Source/JavaScriptCore/dfg/DFGUseKind.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -54,6 +54,7 @@
StringOrStringObjectUse,
NotCellUse,
OtherUse,
+ MiscUse,
LastUseKind // Must always be the last entry in the enum, as it is used to denote the number of enum elements.
};
@@ -96,6 +97,8 @@
return ~SpecCell;
case OtherUse:
return SpecOther;
+ case MiscUse:
+ return SpecMisc;
default:
RELEASE_ASSERT_NOT_REACHED();
return SpecFullTop;
