Diff
Modified: branches/safari-605-branch/Source/_javascript_Core/ChangeLog (228840 => 228841)
--- branches/safari-605-branch/Source/_javascript_Core/ChangeLog 2018-02-20 22:30:10 UTC (rev 228840)
+++ branches/safari-605-branch/Source/_javascript_Core/ChangeLog 2018-02-20 22:30:13 UTC (rev 228841)
@@ -1,5 +1,49 @@
2018-02-20 Jason Marcell <[email protected]>
+ Cherry-pick r228720. rdar://problem/37714022
+
+ 2018-02-19 Saam Barati <[email protected]>
+
+ Don't mark an array profile out of bounds for the cases where the DFG will convert the access to SaneChain
+ https://bugs.webkit.org/show_bug.cgi?id=182912
+ <rdar://problem/37685083>
+
+ Reviewed by Keith Miller.
+
+ In the baseline JIT and LLInt, when we loading a hole from an original array,
+ with the array prototype chain being normal, we end up marking the ArrayProfile
+ for that GetByVal as out of bounds. However, the DFG knows exactly how to
+ optimize this case by returning undefined when loading from a hole. Currently,
+ it only does this for Contiguous arrays (and sometimes Double arrays).
+ This patch just makes sure to not mark the ArrayProfile as out of bounds
+ in this scenario for Contiguous arrays, since the DFG will always optimize
+ this case.
+
+ However, we should extend this by profiling when a GetByVal loads a hole. By
+ doing so, we can optimize this for Int32, ArrayStorage, and maybe even Double
+ arrays. That work will happen in:
+ https://bugs.webkit.org/show_bug.cgi?id=182940
+
+ This patch is a 30-50% speedup on JetStream's hash-map test. This patch
+ speeds up JetStream by 1% when testing on my iMac.
+
+ * dfg/DFGArrayMode.cpp:
+ (JSC::DFG::ArrayMode::refine const):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ * jit/JITOperations.cpp:
+ (JSC::getByVal):
+ (JSC::canAccessArgumentIndexQuickly): Deleted.
+ * llint/LLIntSlowPaths.cpp:
+ (JSC::LLInt::getByVal):
+ (JSC::LLInt::LLINT_SLOW_PATH_DECL):
+ * llint/LowLevelInterpreter32_64.asm:
+ * llint/LowLevelInterpreter64.asm:
+ * runtime/CommonSlowPaths.h:
+ (JSC::CommonSlowPaths::canAccessArgumentIndexQuickly):
+
+2018-02-20 Jason Marcell <[email protected]>
+
Cherry-pick r228693. rdar://problem/37697679
2018-02-17 Filip Pizlo <[email protected]>
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGArrayMode.cpp (228840 => 228841)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGArrayMode.cpp 2018-02-20 22:30:10 UTC (rev 228840)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGArrayMode.cpp 2018-02-20 22:30:13 UTC (rev 228841)
@@ -210,12 +210,16 @@
// If we have an OriginalArray and the JSArray prototype chain is sane,
// any indexed access always return undefined. We have a fast path for that.
JSGlobalObject* globalObject = graph.globalObjectFor(node->origin.semantic);
+ Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure();
+ Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure();
if ((node->op() == GetByVal || canBecomeGetArrayLength(graph, node))
&& arrayClass() == Array::OriginalArray
- && globalObject->arrayPrototypeChainIsSane()
- && !graph.hasExitSite(node->origin.semantic, OutOfBounds)) {
- graph.registerAndWatchStructureTransition(globalObject->arrayPrototype()->structure());
- graph.registerAndWatchStructureTransition(globalObject->objectPrototype()->structure());
+ && !graph.hasExitSite(node->origin.semantic, OutOfBounds)
+ && arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
+ && objectPrototypeStructure->transitionWatchpointSetIsStillValid()
+ && globalObject->arrayPrototypeChainIsSane()) {
+ graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
+ graph.registerAndWatchStructureTransition(objectPrototypeStructure);
if (globalObject->arrayPrototypeChainIsSane())
return withSpeculation(Array::SaneChain);
}
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (228840 => 228841)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2018-02-20 22:30:10 UTC (rev 228840)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2018-02-20 22:30:13 UTC (rev 228841)
@@ -722,55 +722,58 @@
case Array::Double:
if (arrayMode.arrayClass() == Array::OriginalArray
&& arrayMode.speculation() == Array::InBounds) {
- JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
- if (globalObject->arrayPrototypeChainIsSane()) {
- // Check if SaneChain will work on a per-type basis. Note that:
- //
- // 1) We don't want double arrays to sometimes return undefined, since
- // that would require a change to the return type and it would pessimise
- // things a lot. So, we'd only want to do that if we actually had
- // evidence that we could read from a hole. That's pretty annoying.
- // Likely the best way to handle that case is with an equivalent of
- // SaneChain for OutOfBounds. For now we just detect when Undefined and
- // NaN are indistinguishable according to backwards propagation, and just
- // use SaneChain in that case. This happens to catch a lot of cases.
- //
- // 2) We don't want int32 array loads to have to do a hole check just to
- // coerce to Undefined, since that would mean twice the checks.
- //
- // This has two implications. First, we have to do more checks than we'd
- // like. It's unfortunate that we have to do the hole check. Second,
- // some accesses that hit a hole will now need to take the full-blown
- // out-of-bounds slow path. We can fix that with:
- // https://bugs.webkit.org/show_bug.cgi?id=144668
+ // Check if SaneChain will work on a per-type basis. Note that:
+ //
+ // 1) We don't want double arrays to sometimes return undefined, since
+ // that would require a change to the return type and it would pessimise
+ // things a lot. So, we'd only want to do that if we actually had
+ // evidence that we could read from a hole. That's pretty annoying.
+ // Likely the best way to handle that case is with an equivalent of
+ // SaneChain for OutOfBounds. For now we just detect when Undefined and
+ // NaN are indistinguishable according to backwards propagation, and just
+ // use SaneChain in that case. This happens to catch a lot of cases.
+ //
+ // 2) We don't want int32 array loads to have to do a hole check just to
+ // coerce to Undefined, since that would mean twice the checks.
+ //
+ // This has two implications. First, we have to do more checks than we'd
+ // like. It's unfortunate that we have to do the hole check. Second,
+ // some accesses that hit a hole will now need to take the full-blown
+ // out-of-bounds slow path. We can fix that with:
+ // https://bugs.webkit.org/show_bug.cgi?id=144668
+
+ bool canDoSaneChain = false;
+ switch (arrayMode.type()) {
+ case Array::Contiguous:
+ // This is happens to be entirely natural. We already would have
+ // returned any JSValue, and now we'll return Undefined. We still do
+ // the check but it doesn't require taking any kind of slow path.
+ canDoSaneChain = true;
+ break;
- bool canDoSaneChain = false;
- switch (arrayMode.type()) {
- case Array::Contiguous:
- // This is happens to be entirely natural. We already would have
- // returned any JSValue, and now we'll return Undefined. We still do
- // the check but it doesn't require taking any kind of slow path.
+ case Array::Double:
+ if (!(node->flags() & NodeBytecodeUsesAsOther)) {
+ // Holes look like NaN already, so if the user doesn't care
+ // about the difference between Undefined and NaN then we can
+ // do this.
canDoSaneChain = true;
- break;
-
- case Array::Double:
- if (!(node->flags() & NodeBytecodeUsesAsOther)) {
- // Holes look like NaN already, so if the user doesn't care
- // about the difference between Undefined and NaN then we can
- // do this.
- canDoSaneChain = true;
- }
- break;
-
- default:
- break;
}
+ break;
- if (canDoSaneChain) {
- m_graph.registerAndWatchStructureTransition(globalObject->arrayPrototype()->structure());
- m_graph.registerAndWatchStructureTransition(globalObject->objectPrototype()->structure());
- if (globalObject->arrayPrototypeChainIsSane())
- node->setArrayMode(arrayMode.withSpeculation(Array::SaneChain));
+ default:
+ break;
+ }
+
+ if (canDoSaneChain) {
+ JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
+ Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure();
+ Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure();
+ if (arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
+ && objectPrototypeStructure->transitionWatchpointSetIsStillValid()
+ && globalObject->arrayPrototypeChainIsSane()) {
+ m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
+ m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
+ node->setArrayMode(arrayMode.withSpeculation(Array::SaneChain));
}
}
}
Modified: branches/safari-605-branch/Source/_javascript_Core/jit/JITOperations.cpp (228840 => 228841)
--- branches/safari-605-branch/Source/_javascript_Core/jit/JITOperations.cpp 2018-02-20 22:30:10 UTC (rev 228840)
+++ branches/safari-605-branch/Source/_javascript_Core/jit/JITOperations.cpp 2018-02-20 22:30:13 UTC (rev 228841)
@@ -1724,27 +1724,6 @@
}
-static bool canAccessArgumentIndexQuickly(JSObject& object, uint32_t index)
-{
- switch (object.structure()->typeInfo().type()) {
- case DirectArgumentsType: {
- DirectArguments* directArguments = jsCast<DirectArguments*>(&object);
- if (directArguments->isMappedArgumentInDFG(index))
- return true;
- break;
- }
- case ScopedArgumentsType: {
- ScopedArguments* scopedArguments = jsCast<ScopedArguments*>(&object);
- if (scopedArguments->isMappedArgumentInDFG(index))
- return true;
- break;
- }
- default:
- break;
- }
- return false;
-}
-
static JSValue getByVal(ExecState* exec, JSValue baseValue, JSValue subscript, ByValInfo* byValInfo, ReturnAddressPtr returnAddress)
{
VM& vm = exec->vm();
@@ -1781,7 +1760,16 @@
if (object->canGetIndexQuickly(i))
return object->getIndexQuickly(i);
- if (!canAccessArgumentIndexQuickly(*object, i)) {
+ bool skipMarkingOutOfBounds = false;
+
+ if (object->indexingType() == ArrayWithContiguous && i < object->butterfly()->publicLength()) {
+ // FIXME: expand this to ArrayStorage, Int32, and maybe Double:
+ // https://bugs.webkit.org/show_bug.cgi?id=182940
+ auto* globalObject = object->globalObject();
+ skipMarkingOutOfBounds = globalObject->isOriginalArrayStructure(object->structure()) && globalObject->arrayPrototypeChainIsSane();
+ }
+
+ if (!skipMarkingOutOfBounds && !CommonSlowPaths::canAccessArgumentIndexQuickly(*object, i)) {
// FIXME: This will make us think that in-bounds typed array accesses are actually
// out-of-bounds.
// https://bugs.webkit.org/show_bug.cgi?id=149886
@@ -1950,7 +1938,7 @@
if (object->canGetIndexQuickly(index))
return JSValue::encode(JSValue(JSValue::JSTrue));
- if (!canAccessArgumentIndexQuickly(*object, index)) {
+ if (!CommonSlowPaths::canAccessArgumentIndexQuickly(*object, index)) {
// FIXME: This will make us think that in-bounds typed array accesses are actually
// out-of-bounds.
// https://bugs.webkit.org/show_bug.cgi?id=149886
@@ -1974,7 +1962,7 @@
if (object->canGetIndexQuickly(index))
return JSValue::encode(JSValue(JSValue::JSTrue));
- if (!canAccessArgumentIndexQuickly(*object, index)) {
+ if (!CommonSlowPaths::canAccessArgumentIndexQuickly(*object, index)) {
// FIXME: This will make us think that in-bounds typed array accesses are actually
// out-of-bounds.
// https://bugs.webkit.org/show_bug.cgi?id=149886
Modified: branches/safari-605-branch/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (228840 => 228841)
--- branches/safari-605-branch/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2018-02-20 22:30:10 UTC (rev 228840)
+++ branches/safari-605-branch/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2018-02-20 22:30:13 UTC (rev 228841)
@@ -836,7 +836,7 @@
LLINT_RETURN(jsBoolean(couldDelete));
}
-static ALWAYS_INLINE JSValue getByVal(VM& vm, ExecState* exec, JSValue baseValue, JSValue subscript)
+static ALWAYS_INLINE JSValue getByVal(VM& vm, ExecState* exec, Instruction* pc, JSValue baseValue, JSValue subscript)
{
auto scope = DECLARE_THROW_SCOPE(vm);
@@ -852,10 +852,32 @@
if (subscript.isUInt32()) {
uint32_t i = subscript.asUInt32();
- if (isJSString(baseValue) && asString(baseValue)->canGetIndex(i)) {
- scope.release();
- return asString(baseValue)->getIndex(exec, i);
+ ArrayProfile* arrayProfile = pc[4].u.arrayProfile;
+
+ if (isJSString(baseValue)) {
+ if (asString(baseValue)->canGetIndex(i)) {
+ scope.release();
+ return asString(baseValue)->getIndex(exec, i);
+ }
+ arrayProfile->setOutOfBounds();
+ } else if (baseValue.isObject()) {
+ JSObject* object = asObject(baseValue);
+ if (object->canGetIndexQuickly(i))
+ return object->getIndexQuickly(i);
+
+ bool skipMarkingOutOfBounds = false;
+
+ if (object->indexingType() == ArrayWithContiguous && i < object->butterfly()->publicLength()) {
+ // FIXME: expand this to ArrayStorage, Int32, and maybe Double:
+ // https://bugs.webkit.org/show_bug.cgi?id=182940
+ auto* globalObject = object->globalObject();
+ skipMarkingOutOfBounds = globalObject->isOriginalArrayStructure(object->structure()) && globalObject->arrayPrototypeChainIsSane();
+ }
+
+ if (!skipMarkingOutOfBounds && !CommonSlowPaths::canAccessArgumentIndexQuickly(*object, i))
+ arrayProfile->setOutOfBounds();
}
+
scope.release();
return baseValue.get(exec, i);
}
@@ -871,7 +893,7 @@
LLINT_SLOW_PATH_DECL(slow_path_get_by_val)
{
LLINT_BEGIN();
- LLINT_RETURN_PROFILED(op_get_by_val, getByVal(vm, exec, LLINT_OP_C(2).jsValue(), LLINT_OP_C(3).jsValue()));
+ LLINT_RETURN_PROFILED(op_get_by_val, getByVal(vm, exec, pc, LLINT_OP_C(2).jsValue(), LLINT_OP_C(3).jsValue()));
}
LLINT_SLOW_PATH_DECL(slow_path_put_by_val)
Modified: branches/safari-605-branch/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm (228840 => 228841)
--- branches/safari-605-branch/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2018-02-20 22:30:10 UTC (rev 228840)
+++ branches/safari-605-branch/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2018-02-20 22:30:13 UTC (rev 228841)
@@ -1583,7 +1583,7 @@
bineq t2, ContiguousShape, .opGetByValNotContiguous
.opGetByValIsContiguous:
- biaeq t1, -sizeof IndexingHeader + IndexingHeader::u.lengths.publicLength[t3], .opGetByValOutOfBounds
+ biaeq t1, -sizeof IndexingHeader + IndexingHeader::u.lengths.publicLength[t3], .opGetByValSlow
andi JSObject::m_butterflyIndexingMask[t0], t1
loadi TagOffset[t3, t1, 8], t2
loadi PayloadOffset[t3, t1, 8], t1
@@ -1591,7 +1591,7 @@
.opGetByValNotContiguous:
bineq t2, DoubleShape, .opGetByValNotDouble
- biaeq t1, -sizeof IndexingHeader + IndexingHeader::u.lengths.publicLength[t3], .opGetByValOutOfBounds
+ biaeq t1, -sizeof IndexingHeader + IndexingHeader::u.lengths.publicLength[t3], .opGetByValSlow
andi JSObject::m_butterflyIndexingMask[t0], t1
loadd [t3, t1, 8], ft0
bdnequn ft0, ft0, .opGetByValSlow
@@ -1603,7 +1603,7 @@
.opGetByValNotDouble:
subi ArrayStorageShape, t2
bia t2, SlowPutArrayStorageShape - ArrayStorageShape, .opGetByValSlow
- biaeq t1, -sizeof IndexingHeader + IndexingHeader::u.lengths.vectorLength[t3], .opGetByValOutOfBounds
+ biaeq t1, -sizeof IndexingHeader + IndexingHeader::u.lengths.vectorLength[t3], .opGetByValSlow
andi JSObject::m_butterflyIndexingMask[t0], t1
loadi ArrayStorage::m_vector + TagOffset[t3, t1, 8], t2
loadi ArrayStorage::m_vector + PayloadOffset[t3, t1, 8], t1
@@ -1610,7 +1610,7 @@
.opGetByValDone:
loadi 4[PC], t0
- bieq t2, EmptyValueTag, .opGetByValOutOfBounds
+ bieq t2, EmptyValueTag, .opGetByValSlow
.opGetByValNotEmpty:
storei t2, TagOffset[cfr, t0, 8]
storei t1, PayloadOffset[cfr, t0, 8]
@@ -1617,9 +1617,6 @@
valueProfile(t2, t1, 20, t0)
dispatch(constexpr op_get_by_val_length)
-.opGetByValOutOfBounds:
- loadpFromInstruction(4, t0)
- storeb 1, ArrayProfile::m_outOfBounds[t0]
.opGetByValSlow:
callOpcodeSlowPath(_llint_slow_path_get_by_val)
dispatch(constexpr op_get_by_val_length)
Modified: branches/safari-605-branch/Source/_javascript_Core/llint/LowLevelInterpreter64.asm (228840 => 228841)
--- branches/safari-605-branch/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2018-02-20 22:30:10 UTC (rev 228840)
+++ branches/safari-605-branch/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2018-02-20 22:30:13 UTC (rev 228841)
@@ -1514,20 +1514,20 @@
bineq t2, ContiguousShape, .opGetByValNotContiguous
.opGetByValIsContiguous:
- biaeq t1, -sizeof IndexingHeader + IndexingHeader::u.lengths.publicLength[t3], .opGetByValOutOfBounds
+ biaeq t1, -sizeof IndexingHeader + IndexingHeader::u.lengths.publicLength[t3], .opGetByValSlow
andi JSObject::m_butterflyIndexingMask[t0], t1
loadisFromInstruction(1, t0)
loadq [t3, t1, 8], t2
- btqz t2, .opGetByValOutOfBounds
+ btqz t2, .opGetByValSlow
jmp .opGetByValDone
.opGetByValNotContiguous:
bineq t2, DoubleShape, .opGetByValNotDouble
- biaeq t1, -sizeof IndexingHeader + IndexingHeader::u.lengths.publicLength[t3], .opGetByValOutOfBounds
+ biaeq t1, -sizeof IndexingHeader + IndexingHeader::u.lengths.publicLength[t3], .opGetByValSlow
andi JSObject::m_butterflyIndexingMask[t0], t1
loadisFromInstruction(1 ,t0)
loadd [t3, t1, 8], ft0
- bdnequn ft0, ft0, .opGetByValOutOfBounds
+ bdnequn ft0, ft0, .opGetByValSlow
fd2q ft0, t2
subq tagTypeNumber, t2
jmp .opGetByValDone
@@ -1535,11 +1535,11 @@
.opGetByValNotDouble:
subi ArrayStorageShape, t2
bia t2, SlowPutArrayStorageShape - ArrayStorageShape, .opGetByValNotIndexedStorage
- biaeq t1, -sizeof IndexingHeader + IndexingHeader::u.lengths.vectorLength[t3], .opGetByValOutOfBounds
+ biaeq t1, -sizeof IndexingHeader + IndexingHeader::u.lengths.vectorLength[t3], .opGetByValSlow
andi JSObject::m_butterflyIndexingMask[t0], t1
loadisFromInstruction(1, t0)
loadq ArrayStorage::m_vector[t3, t1, 8], t2
- btqz t2, .opGetByValOutOfBounds
+ btqz t2, .opGetByValSlow
.opGetByValDone:
storeq t2, [cfr, t0, 8]
@@ -1546,11 +1546,6 @@
valueProfile(t2, 5, t0)
dispatch(constexpr op_get_by_val_length)
-.opGetByValOutOfBounds:
- loadpFromInstruction(4, t0)
- storeb 1, ArrayProfile::m_outOfBounds[t0]
- jmp .opGetByValSlow
-
.opGetByValNotIndexedStorage:
# First lets check if we even have a typed array. This lets us do some boilerplate up front.
loadb JSCell::m_type[t0], t2
Modified: branches/safari-605-branch/Source/_javascript_Core/runtime/CommonSlowPaths.h (228840 => 228841)
--- branches/safari-605-branch/Source/_javascript_Core/runtime/CommonSlowPaths.h 2018-02-20 22:30:10 UTC (rev 228840)
+++ branches/safari-605-branch/Source/_javascript_Core/runtime/CommonSlowPaths.h 2018-02-20 22:30:13 UTC (rev 228841)
@@ -27,8 +27,10 @@
#include "CodeBlock.h"
#include "CodeSpecializationKind.h"
+#include "DirectArguments.h"
#include "ExceptionHelpers.h"
#include "FunctionCodeBlock.h"
+#include "ScopedArguments.h"
#include "SlowPathReturnType.h"
#include "StackAlignment.h"
#include "VMInlines.h"
@@ -202,6 +204,27 @@
}
}
+inline bool canAccessArgumentIndexQuickly(JSObject& object, uint32_t index)
+{
+ switch (object.structure()->typeInfo().type()) {
+ case DirectArgumentsType: {
+ DirectArguments* directArguments = jsCast<DirectArguments*>(&object);
+ if (directArguments->isMappedArgumentInDFG(index))
+ return true;
+ break;
+ }
+ case ScopedArgumentsType: {
+ ScopedArguments* scopedArguments = jsCast<ScopedArguments*>(&object);
+ if (scopedArguments->isMappedArgumentInDFG(index))
+ return true;
+ break;
+ }
+ default:
+ break;
+ }
+ return false;
+}
+
} // namespace CommonSlowPaths
class ExecState;
