Diff
Modified: trunk/JSTests/ChangeLog (265774 => 265775)
--- trunk/JSTests/ChangeLog 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/JSTests/ChangeLog 2020-08-17 22:10:14 UTC (rev 265775)
@@ -1,3 +1,16 @@
+2020-08-17 Saam Barati <[email protected]>
+
+ Have an OOB+SaneChain Array::Speculation
+ https://bugs.webkit.org/show_bug.cgi?id=215487
+
+ Reviewed by Yusuke Suzuki.
+
+ * microbenchmarks/oob-sane-chain-contiguous.js: Added.
+ * microbenchmarks/oob-sane-chain-double-read-undefined.js: Added.
+ * microbenchmarks/oob-sane-chain-double.js: Added.
+ * microbenchmarks/oob-sane-chain-int32.js: Added.
+ * stress/oob-sane-chain-negative-index.js: Added.
+
2020-08-16 Yusuke Suzuki <[email protected]>
Unreviewed, reduce iteration count more for non-JIT environments
Added: trunk/JSTests/microbenchmarks/oob-sane-chain-contiguous.js (0 => 265775)
--- trunk/JSTests/microbenchmarks/oob-sane-chain-contiguous.js (rev 0)
+++ trunk/JSTests/microbenchmarks/oob-sane-chain-contiguous.js 2020-08-17 22:10:14 UTC (rev 265775)
@@ -0,0 +1,18 @@
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+
+function bar(a, x) {
+ return a[x];
+}
+noInline(bar);
+
+let o = {};
+let b = [o, "", , 25];
+
+for (let i = 0; i < 10000000; ++i) {
+ assert(bar(b, 0) === o);
+ assert(bar(b, 5) === undefined);
+ assert(bar(b, 2) === undefined);
+}
Added: trunk/JSTests/microbenchmarks/oob-sane-chain-double-read-undefined.js (0 => 265775)
--- trunk/JSTests/microbenchmarks/oob-sane-chain-double-read-undefined.js (rev 0)
+++ trunk/JSTests/microbenchmarks/oob-sane-chain-double-read-undefined.js 2020-08-17 22:10:14 UTC (rev 265775)
@@ -0,0 +1,18 @@
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+
+function baz(a, x) {
+ return a[x];
+}
+noInline(baz);
+
+let a = [1.5, 2.5, , 3.5];
+
+for (let i = 0; i < 10000000; ++i) {
+ assert(baz(a, 0) === 1.5);
+ assert(baz(a, 5) === undefined);
+ assert(baz(a, 2) === undefined);
+}
+
Added: trunk/JSTests/microbenchmarks/oob-sane-chain-double.js (0 => 265775)
--- trunk/JSTests/microbenchmarks/oob-sane-chain-double.js (rev 0)
+++ trunk/JSTests/microbenchmarks/oob-sane-chain-double.js 2020-08-17 22:10:14 UTC (rev 265775)
@@ -0,0 +1,17 @@
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+
+function foo(a, x) {
+ return a[x] * 2.5;
+}
+noInline(foo);
+
+let a = [1.5, 2.5, , 3.5];
+
+for (let i = 0; i < 10000000; ++i) {
+ assert(foo(a, 0) === 2.5 * 1.5);
+ assert(isNaN(foo(a, 5)));
+ assert(isNaN(foo(a, 2)));
+}
Added: trunk/JSTests/microbenchmarks/oob-sane-chain-int32.js (0 => 265775)
--- trunk/JSTests/microbenchmarks/oob-sane-chain-int32.js (rev 0)
+++ trunk/JSTests/microbenchmarks/oob-sane-chain-int32.js 2020-08-17 22:10:14 UTC (rev 265775)
@@ -0,0 +1,17 @@
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+
+function bar(a, x) {
+ return a[x];
+}
+noInline(bar);
+
+let b = [0, 1, , 2];
+
+for (let i = 0; i < 10000000; ++i) {
+ assert(bar(b, 0) === 0);
+ assert(bar(b, 5) === undefined);
+ assert(bar(b, 2) === undefined);
+}
Added: trunk/JSTests/stress/oob-sane-chain-negative-index.js (0 => 265775)
--- trunk/JSTests/stress/oob-sane-chain-negative-index.js (rev 0)
+++ trunk/JSTests/stress/oob-sane-chain-negative-index.js 2020-08-17 22:10:14 UTC (rev 265775)
@@ -0,0 +1,24 @@
+let called = false;
+Object.defineProperty(Array.prototype, "-1", {
+ get: function() {
+ called = true;
+ return 42;
+ }
+});
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+
+function baz(a, x) {
+ return a[x];
+}
+noInline(baz);
+
+let a = [1,2,3];
+
+for (let i = 0; i < 10000000; ++i) {
+ assert(baz(a, -1) === 42);
+ assert(called);
+ called = false;
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (265774 => 265775)
--- trunk/Source/_javascript_Core/ChangeLog 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-08-17 22:10:14 UTC (rev 265775)
@@ -1,3 +1,57 @@
+2020-08-17 Saam Barati <[email protected]>
+
+ Have an OOB+SaneChain Array::Speculation
+ https://bugs.webkit.org/show_bug.cgi?id=215487
+
+ Reviewed by Yusuke Suzuki.
+
+ This patch adds a new ArrayMode speculation in the DFG/FTL called OutOfBoundsSaneChain.
+ It allows us to do fast things when we go OOB, like simply return undefined.
+ This is because we install watchpoints on the prototype chain to ensure they
+ have no indexed properties. This patch implements OutOfBoundsSaneChain on
+ GetByVal over Int32/Double/Contiguous original JS arrays. We can extend it in
+ the future to non original JS arrays if we prove their prototype is Array.prototype.
+ To implement this properly, we also need to ensure that the index isn't negative,
+ as Array.prototype/Object.prototype may have negative indexed accessors. We
+ do this via speculation, and if we ever recompile, and see an exit because of
+ this, we will stop speculating OutOfBoundsSaneChain.
+
+ This is about 20% faster on crypto-md5-SP. And ~3-4x faster on the
+ microbenchmarks I created.
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGArrayMode.cpp:
+ (JSC::DFG::ArrayMode::refine const):
+ (JSC::DFG::arraySpeculationToString):
+ * dfg/DFGArrayMode.h:
+ (JSC::DFG::ArrayMode::isInBoundsSaneChain const):
+ (JSC::DFG::ArrayMode::isOutOfBoundsSaneChain const):
+ (JSC::DFG::ArrayMode::isOutOfBounds const):
+ (JSC::DFG::ArrayMode::isEffectfulOutOfBounds const):
+ (JSC::DFG::ArrayMode::isInBounds const):
+ (JSC::DFG::ArrayMode::isSaneChain const): Deleted.
+ * dfg/DFGCSEPhase.cpp:
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ (JSC::DFG::FixupPhase::checkArray):
+ (JSC::DFG::FixupPhase::setSaneChainIfPossible):
+ (JSC::DFG::FixupPhase::convertToHasIndexedProperty):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileGetByValOnString):
+ (JSC::DFG::SpeculativeJIT::compileHasIndexedProperty):
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGValidate.cpp:
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
+ (JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
+ (JSC::FTL::DFG::LowerDFGToB3::compileHasIndexedProperty):
+
2020-08-16 Alexey Shvayka <[email protected]>
Remove OpIsObjectOrNull from ClassExprNode::emitBytecode()
Modified: trunk/Source/_javascript_Core/bytecode/ExitKind.cpp (265774 => 265775)
--- trunk/Source/_javascript_Core/bytecode/ExitKind.cpp 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/bytecode/ExitKind.cpp 2020-08-17 22:10:14 UTC (rev 265775)
@@ -56,6 +56,8 @@
return "Overflow";
case NegativeZero:
return "NegativeZero";
+ case NegativeIndex:
+ return "NegativeIndex";
case Int52Overflow:
return "Int52Overflow";
case StoreToHole:
Modified: trunk/Source/_javascript_Core/bytecode/ExitKind.h (265774 => 265775)
--- trunk/Source/_javascript_Core/bytecode/ExitKind.h 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/bytecode/ExitKind.h 2020-08-17 22:10:14 UTC (rev 265775)
@@ -39,6 +39,7 @@
BadTypeInfoFlags, // We exited because we made an incorrect assumption about what TypeInfo flags we would see.
Overflow, // We exited because of overflow.
NegativeZero, // We exited because we encountered negative zero.
+ NegativeIndex, // We exited because we encountered a negative index in a place we didn't want to see it.
Int52Overflow, // We exited because of an Int52 overflow.
StoreToHole, // We had a store to a hole.
LoadFromHole, // We had a load from a hole.
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (265774 => 265775)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2020-08-17 22:10:14 UTC (rev 265775)
@@ -2350,7 +2350,7 @@
// we got to the backend. So to do this right, we'd have to get the
// fixup phase to check the watchpoint state and then bake into the
// GetByVal operation the fact that we're using a watchpoint, using
- // something like Array::SaneChain (except not quite, because that
+ // something like Array::InBoundsSaneChain (except not quite, because that
// implies an in-bounds access). None of this feels like it's worth it,
// so we're going with TOP for now. The same thing applies to
// clobbering the world.
@@ -2366,25 +2366,32 @@
makeHeapTopForNode(node);
break;
case Array::Int32:
- if (node->arrayMode().isOutOfBounds()) {
+ if (node->arrayMode().isEffectfulOutOfBounds()) {
clobberWorld();
makeHeapTopForNode(node);
- } else
+ } else if (node->arrayMode().isOutOfBoundsSaneChain())
+ setNonCellTypeForNode(node, SpecInt32Only | SpecOther);
+ else
setNonCellTypeForNode(node, SpecInt32Only);
break;
case Array::Double:
- if (node->arrayMode().isOutOfBounds()) {
+ if (node->arrayMode().isEffectfulOutOfBounds()) {
clobberWorld();
makeHeapTopForNode(node);
- } else if (node->arrayMode().isSaneChain())
+ } else if (node->arrayMode().isInBoundsSaneChain())
setNonCellTypeForNode(node, SpecBytecodeDouble);
- else
+ else if (node->arrayMode().isOutOfBoundsSaneChain()) {
+ if (!!(node->flags() & NodeBytecodeUsesAsOther))
+ setNonCellTypeForNode(node, SpecBytecodeDouble | SpecOther);
+ else
+ setNonCellTypeForNode(node, SpecBytecodeDouble);
+ } else
setNonCellTypeForNode(node, SpecDoubleReal);
break;
case Array::Contiguous:
case Array::ArrayStorage:
case Array::SlowPutArrayStorage:
- if (node->arrayMode().isOutOfBounds())
+ if (node->arrayMode().isEffectfulOutOfBounds())
clobberWorld();
makeHeapTopForNode(node);
break;
Modified: trunk/Source/_javascript_Core/dfg/DFGArrayMode.cpp (265774 => 265775)
--- trunk/Source/_javascript_Core/dfg/DFGArrayMode.cpp 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/dfg/DFGArrayMode.cpp 2020-08-17 22:10:14 UTC (rev 265775)
@@ -260,7 +260,7 @@
graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
graph.registerAndWatchStructureTransition(objectPrototypeStructure);
if (globalObject->arrayPrototypeChainIsSane())
- return withSpeculation(Array::SaneChain);
+ return withSpeculation(Array::InBoundsSaneChain);
}
return ArrayMode(Array::Generic, action());
}
@@ -713,8 +713,8 @@
const char* arraySpeculationToString(Array::Speculation speculation)
{
switch (speculation) {
- case Array::SaneChain:
- return "SaneChain";
+ case Array::InBoundsSaneChain:
+ return "InBoundsSaneChain";
case Array::InBounds:
return "InBounds";
case Array::ToHole:
@@ -721,6 +721,8 @@
return "ToHole";
case Array::OutOfBounds:
return "OutOfBounds";
+ case Array::OutOfBoundsSaneChain:
+ return "OutOfBoundsSaneChain";
default:
return "Unknown!";
}
Modified: trunk/Source/_javascript_Core/dfg/DFGArrayMode.h (265774 => 265775)
--- trunk/Source/_javascript_Core/dfg/DFGArrayMode.h 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/dfg/DFGArrayMode.h 2020-08-17 22:10:14 UTC (rev 265775)
@@ -89,10 +89,11 @@
};
enum Speculation : uint8_t {
- SaneChain, // In bounds and the array prototype chain is still intact, i.e. loading a hole doesn't require special treatment.
+ InBoundsSaneChain, // In bounds and the array prototype chain is still intact, i.e. loading a hole doesn't require special treatment.
InBounds, // In bounds and not loading a hole.
ToHole, // Potentially storing to a hole.
+ OutOfBoundsSaneChain, // Out-of-bounds access, but sane chain, so there are no arbitrary effects. E.g, loading out of bounds doesn't require traversing the prototype chain if we're an original array structure.
OutOfBounds // Out-of-bounds access and anything can happen.
};
enum Conversion : uint8_t {
@@ -276,15 +277,35 @@
return arrayClass() == Array::OriginalArray || arrayClass() == Array::OriginalCopyOnWriteArray;
}
- bool isSaneChain() const
+ bool isInBoundsSaneChain() const
{
- return speculation() == Array::SaneChain;
+ return speculation() == Array::InBoundsSaneChain;
}
+
+ bool isOutOfBoundsSaneChain() const
+ {
+ return speculation() == Array::OutOfBoundsSaneChain;
+ }
+
+ bool isAnySaneChain() const
+ {
+ return isInBoundsSaneChain() || isOutOfBoundsSaneChain();
+ }
+ bool isOutOfBounds() const
+ {
+ return speculation() == Array::OutOfBounds || speculation() == Array::OutOfBoundsSaneChain;
+ }
+
+ bool isEffectfulOutOfBounds() const
+ {
+ return speculation() == Array::OutOfBounds;
+ }
+
bool isInBounds() const
{
switch (speculation()) {
- case Array::SaneChain:
+ case Array::InBoundsSaneChain:
case Array::InBounds:
return true;
default:
@@ -297,11 +318,6 @@
return !isInBounds();
}
- bool isOutOfBounds() const
- {
- return speculation() == Array::OutOfBounds;
- }
-
bool isSlowPut() const
{
return type() == Array::SlowPutArrayStorage;
Modified: trunk/Source/_javascript_Core/dfg/DFGCSEPhase.cpp (265774 => 265775)
--- trunk/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2020-08-17 22:10:14 UTC (rev 265775)
@@ -550,7 +550,7 @@
case Array::Double: {
if (!mode.isInBounds())
break;
- LocationKind kind = mode.isSaneChain() ? IndexedPropertyDoubleSaneChainLoc : IndexedPropertyDoubleLoc;
+ LocationKind kind = mode.isInBoundsSaneChain() ? IndexedPropertyDoubleSaneChainLoc : IndexedPropertyDoubleLoc;
heap = HeapLocation(kind, IndexedDoubleProperties, base, index);
break;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGClobberize.h (265774 => 265775)
--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2020-08-17 22:10:14 UTC (rev 265775)
@@ -929,10 +929,11 @@
return;
case Array::Int32:
- if (mode.isInBounds()) {
+ if (mode.isInBounds() || mode.isOutOfBoundsSaneChain()) {
read(Butterfly_publicLength);
read(IndexedInt32Properties);
- def(HeapLocation(indexedPropertyLoc, IndexedInt32Properties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
+ LocationKind kind = mode.isOutOfBoundsSaneChain() ? IndexedPropertyInt32OrOtherLoc : indexedPropertyLoc;
+ def(HeapLocation(kind, IndexedInt32Properties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
return;
}
clobberTop();
@@ -939,10 +940,16 @@
return;
case Array::Double:
- if (mode.isInBounds()) {
+ if (mode.isInBounds() || mode.isOutOfBoundsSaneChain()) {
read(Butterfly_publicLength);
read(IndexedDoubleProperties);
- LocationKind kind = mode.isSaneChain() ? IndexedPropertyDoubleSaneChainLoc : IndexedPropertyDoubleLoc;
+ LocationKind kind;
+ if (node->hasDoubleResult())
+ kind = mode.isAnySaneChain() ? IndexedPropertyDoubleSaneChainLoc : IndexedPropertyDoubleLoc;
+ else {
+ ASSERT(mode.isOutOfBoundsSaneChain());
+ kind = IndexedPropertyDoubleOrOtherSaneChainLoc;
+ }
def(HeapLocation(kind, IndexedDoubleProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
return;
}
@@ -950,7 +957,7 @@
return;
case Array::Contiguous:
- if (mode.isInBounds()) {
+ if (mode.isInBounds() || mode.isOutOfBoundsSaneChain()) {
read(Butterfly_publicLength);
read(IndexedContiguousProperties);
def(HeapLocation(indexedPropertyLoc, IndexedContiguousProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
@@ -1042,6 +1049,7 @@
if (node->arrayMode().mayStoreToHole())
write(Butterfly_publicLength);
def(HeapLocation(indexedPropertyLoc, IndexedInt32Properties, base, index), LazyNode(value));
+ def(HeapLocation(IndexedPropertyInt32OrOtherLoc, IndexedInt32Properties, base, index), LazyNode(value));
return;
case Array::Double:
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (265774 => 265775)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2020-08-17 22:10:14 UTC (rev 265775)
@@ -1059,8 +1059,10 @@
switch (arrayMode.type()) {
case Array::Contiguous:
case Array::Double:
- if (arrayMode.isJSArrayWithOriginalStructure() && arrayMode.speculation() == Array::InBounds) {
- // Check if SaneChain will work on a per-type basis. Note that:
+ case Array::Int32: {
+ Optional<Array::Speculation> saneChainSpeculation;
+ if (arrayMode.isJSArrayWithOriginalStructure()) {
+ // Check if InBoundsSaneChain 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
@@ -1067,12 +1069,14 @@
// 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
+ // InBoundsSaneChain 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.
+ // use InBoundsSaneChain 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.
+ // coerce to Undefined, since that would mean twice the checks. We want to
+ // be able to say we always return Int32. FIXME: Maybe this should be profiling
+ // based?
//
// 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,
@@ -1080,32 +1084,41 @@
// 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::Int32:
+ if (is64Bit() && arrayMode.speculation() == Array::OutOfBounds && !m_graph.hasExitSite(node->origin.semantic, NegativeIndex))
+ saneChainSpeculation = Array::OutOfBoundsSaneChain;
+ break;
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;
+ if (is64Bit() && arrayMode.speculation() == Array::OutOfBounds && !m_graph.hasExitSite(node->origin.semantic, NegativeIndex))
+ saneChainSpeculation = Array::OutOfBoundsSaneChain;
+ else if (arrayMode.speculation() == Array::InBounds)
+ saneChainSpeculation = Array::InBoundsSaneChain;
break;
case Array::Double:
- if (!(node->flags() & NodeBytecodeUsesAsOther)) {
+ if (!(node->flags() & NodeBytecodeUsesAsOther) && arrayMode.speculation() == Array::InBounds) {
// 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;
- }
+ saneChainSpeculation = Array::InBoundsSaneChain;
+ } else if (is64Bit() && arrayMode.speculation() == Array::OutOfBounds && !m_graph.hasExitSite(node->origin.semantic, NegativeIndex))
+ saneChainSpeculation = Array::OutOfBoundsSaneChain;
break;
default:
break;
}
-
- if (canDoSaneChain)
- setSaneChainIfPossible(node);
}
+
+ if (saneChainSpeculation)
+ setSaneChainIfPossible(node, *saneChainSpeculation);
+
break;
+ }
case Array::String:
if ((node->prediction() & ~SpecString)
@@ -1155,7 +1168,8 @@
switch (arrayMode.type()) {
case Array::Double:
- if (!arrayMode.isOutOfBounds())
+ if (!arrayMode.isOutOfBounds()
+ || (arrayMode.isOutOfBoundsSaneChain() && !(node->flags() & NodeBytecodeUsesAsOther)))
node->setResult(NodeResultDouble);
break;
@@ -2202,7 +2216,7 @@
// FIXME: OutOfBounds shouldn't preclude going sane chain because OOB is just false and cannot have effects.
// See: https://bugs.webkit.org/show_bug.cgi?id=209456
if (arrayMode.isJSArrayWithOriginalStructure() && arrayMode.speculation() == Array::InBounds)
- setSaneChainIfPossible(node);
+ setSaneChainIfPossible(node, Array::InBoundsSaneChain);
break;
}
case GetDirectPname: {
@@ -3509,7 +3523,7 @@
m_insertionSet.insertNode(
m_indexInBlock, SpecNone, Check, origin, Edge(array, StringUse));
} else {
- // Note that we only need to be using a structure check if we opt for SaneChain, since
+ // Note that we only need to be using a structure check if we opt for InBoundsSaneChain, since
// that needs to protect against JSArray's __proto__ being changed.
Structure* structure = arrayMode.originalArrayStructure(m_graph, origin.semantic);
@@ -3551,7 +3565,7 @@
OpInfo(arrayMode.asWord()), Edge(array, KnownCellUse));
}
- void setSaneChainIfPossible(Node* node)
+ void setSaneChainIfPossible(Node* node, Array::Speculation speculation)
{
JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure(vm());
@@ -3561,7 +3575,7 @@
&& globalObject->arrayPrototypeChainIsSane()) {
m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
- node->setArrayMode(node->arrayMode().withSpeculation(Array::SaneChain));
+ node->setArrayMode(node->arrayMode().withSpeculation(speculation));
node->clearFlags(NodeMustGenerate);
}
}
@@ -3943,7 +3957,7 @@
// FIXME: OutOfBounds shouldn't preclude going sane chain because OOB is just false and cannot have effects.
// See: https://bugs.webkit.org/show_bug.cgi?id=209456
if (arrayMode.isJSArrayWithOriginalStructure() && arrayMode.speculation() == Array::InBounds)
- setSaneChainIfPossible(node);
+ setSaneChainIfPossible(node, Array::InBoundsSaneChain);
fixEdge<CellUse>(m_graph.varArgChild(node, 0));
fixEdge<Int32Use>(m_graph.varArgChild(node, 1));
Modified: trunk/Source/_javascript_Core/dfg/DFGHeapLocation.cpp (265774 => 265775)
--- trunk/Source/_javascript_Core/dfg/DFGHeapLocation.cpp 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/dfg/DFGHeapLocation.cpp 2020-08-17 22:10:14 UTC (rev 265775)
@@ -138,10 +138,18 @@
out.print("IndexedPropertyDoubleSaneChainLoc");
return;
+ case IndexedPropertyDoubleOrOtherSaneChainLoc:
+ out.print("IndexedPropertyDoubleOrOtherSaneChainLoc");
+ return;
+
case IndexedPropertyInt32Loc:
out.print("IndexedPropertyInt32Loc");
return;
+ case IndexedPropertyInt32OrOtherLoc:
+ out.print("IndexedPropertyInt32OrOtherLoc");
+ return;
+
case IndexedPropertyInt52Loc:
out.print("IndexedPropertyInt52Loc");
return;
Modified: trunk/Source/_javascript_Core/dfg/DFGHeapLocation.h (265774 => 265775)
--- trunk/Source/_javascript_Core/dfg/DFGHeapLocation.h 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/dfg/DFGHeapLocation.h 2020-08-17 22:10:14 UTC (rev 265775)
@@ -49,7 +49,9 @@
HasIndexedPropertyLoc,
IndexedPropertyDoubleLoc,
IndexedPropertyDoubleSaneChainLoc,
+ IndexedPropertyDoubleOrOtherSaneChainLoc,
IndexedPropertyInt32Loc,
+ IndexedPropertyInt32OrOtherLoc,
IndexedPropertyInt52Loc,
IndexedPropertyJSLoc,
IndexedPropertyStorageLoc,
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (265774 => 265775)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2020-08-17 22:10:14 UTC (rev 265775)
@@ -2408,7 +2408,7 @@
if (globalObject->stringPrototypeChainIsSane()) {
// FIXME: This could be captured using a Speculation mode that means "out-of-bounds
- // loads return a trivial value". Something like SaneChainOutOfBounds. This should
+ // loads return a trivial value". Something like OutOfBoundsSaneChain. This should
// speculate that we don't take negative out-of-bounds, or better yet, it should rely
// on a stringPrototypeChainIsSane() guaranteeing that the prototypes have no negative
// indexed properties either.
@@ -14148,7 +14148,7 @@
m_jit.load32(MacroAssembler::BaseIndex(storageGPR, indexGPR, MacroAssembler::TimesEight, OBJECT_OFFSETOF(JSValue, u.asBits.tag)), scratchGPR);
#endif
- if (mode.isSaneChain()) {
+ if (mode.isInBoundsSaneChain()) {
m_jit.isNotEmpty(scratchGPR, resultGPR);
break;
}
@@ -14177,7 +14177,7 @@
m_jit.loadDouble(MacroAssembler::BaseIndex(storageGPR, indexGPR, MacroAssembler::TimesEight), scratchFPR);
- if (mode.isSaneChain()) {
+ if (mode.isInBoundsSaneChain()) {
m_jit.compareDouble(MacroAssembler::DoubleEqualAndOrdered, scratchFPR, scratchFPR, resultGPR);
break;
}
@@ -14210,13 +14210,13 @@
m_jit.load32(MacroAssembler::BaseIndex(storageGPR, indexGPR, MacroAssembler::TimesEight, ArrayStorage::vectorOffset() + OBJECT_OFFSETOF(JSValue, u.asBits.tag)), scratchGPR);
#endif
- if (mode.isSaneChain()) {
+ if (mode.isInBoundsSaneChain()) {
m_jit.isNotEmpty(scratchGPR, resultGPR);
break;
}
MacroAssembler::Jump isHole = m_jit.branchIfEmpty(scratchGPR);
- if (!mode.isInBounds() || mode.isSaneChain())
+ if (!mode.isInBounds() || mode.isInBoundsSaneChain())
slowCases.append(isHole);
else
speculationCheck(LoadFromHole, JSValueRegs(), nullptr, isHole);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (265774 => 265775)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2020-08-17 22:10:14 UTC (rev 265775)
@@ -2366,7 +2366,7 @@
GPRTemporary resultPayload(this);
if (node->arrayMode().type() == Array::Int32) {
- ASSERT(!node->arrayMode().isSaneChain());
+ ASSERT(!node->arrayMode().isInBoundsSaneChain());
speculationCheck(
OutOfBounds, JSValueRegs(), 0,
@@ -2392,7 +2392,7 @@
MacroAssembler::BaseIndex(
storageReg, propertyReg, MacroAssembler::TimesEight, PayloadOffset),
resultPayload.gpr());
- if (node->arrayMode().isSaneChain()) {
+ if (node->arrayMode().isInBoundsSaneChain()) {
JITCompiler::Jump notHole = m_jit.branchIfNotEmpty(resultTag.gpr());
m_jit.move(TrustedImm32(JSValue::UndefinedTag), resultTag.gpr());
m_jit.move(TrustedImm32(0), resultPayload.gpr());
@@ -2453,7 +2453,7 @@
FPRTemporary result(this);
m_jit.loadDouble(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesEight), result.fpr());
- if (!node->arrayMode().isSaneChain())
+ if (!node->arrayMode().isInBoundsSaneChain())
speculationCheck(LoadFromHole, JSValueRegs(), 0, m_jit.branchIfNaN(result.fpr()));
doubleResult(result.fpr(), node);
break;
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (265774 => 265775)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2020-08-17 22:10:14 UTC (rev 265775)
@@ -2736,7 +2736,7 @@
GPRTemporary result(this);
m_jit.load64(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesEight), result.gpr());
- if (node->arrayMode().isSaneChain()) {
+ if (node->arrayMode().isInBoundsSaneChain()) {
ASSERT(node->arrayMode().type() == Array::Contiguous);
JITCompiler::Jump notHole = m_jit.branchIfNotEmpty(result.gpr());
m_jit.move(TrustedImm64(JSValue::encode(jsUndefined())), result.gpr());
@@ -2769,12 +2769,21 @@
slowCases.append(m_jit.branch32(MacroAssembler::AboveOrEqual, propertyReg, MacroAssembler::Address(storageReg, Butterfly::offsetOfPublicLength())));
m_jit.load64(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesEight), resultReg);
- slowCases.append(m_jit.branchIfEmpty(resultReg));
+
+ if (node->arrayMode().isOutOfBoundsSaneChain()) {
+ auto done = m_jit.branchIfNotEmpty(resultReg);
+ slowCases.link(&m_jit);
+ speculationCheck(NegativeIndex, JSValueRegs(), nullptr, m_jit.branch32(MacroAssembler::LessThan, propertyReg, CCallHelpers::TrustedImm32(0)));
+ m_jit.move(CCallHelpers::TrustedImm64(JSValue::encode(jsUndefined())), resultReg);
+ done.link(&m_jit);
+ } else {
+ slowCases.append(m_jit.branchIfEmpty(resultReg));
+ addSlowPathGenerator(
+ slowPathCall(
+ slowCases, this, operationGetByValObjectInt,
+ result.gpr(), TrustedImmPtr::weakPointer(m_graph, m_graph.globalObjectFor(node->origin.semantic)), baseReg, propertyReg));
+ }
- addSlowPathGenerator(
- slowPathCall(
- slowCases, this, operationGetByValObjectInt,
- result.gpr(), TrustedImmPtr::weakPointer(m_graph, m_graph.globalObjectFor(node->origin.semantic)), baseReg, propertyReg));
jsValueResult(resultReg, node);
break;
@@ -2797,12 +2806,14 @@
speculationCheck(OutOfBounds, JSValueRegs(), nullptr, m_jit.branch32(MacroAssembler::AboveOrEqual, propertyReg, MacroAssembler::Address(storageReg, Butterfly::offsetOfPublicLength())));
m_jit.loadDouble(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesEight), resultReg);
- if (!node->arrayMode().isSaneChain())
+ if (!node->arrayMode().isInBoundsSaneChain())
speculationCheck(LoadFromHole, JSValueRegs(), nullptr, m_jit.branchIfNaN(resultReg));
doubleResult(resultReg, node);
break;
}
+ bool resultIsUnboxed = node->arrayMode().isOutOfBoundsSaneChain() && !(node->flags() & NodeBytecodeUsesAsOther);
+
SpeculateCellOperand base(this, m_graph.varArgChild(node, 0));
SpeculateStrictInt32Operand property(this, m_graph.varArgChild(node, 1));
StorageOperand storage(this, m_graph.varArgChild(node, 2));
@@ -2814,9 +2825,13 @@
if (!m_compileOkay)
return;
- GPRTemporary result(this);
+ Optional<GPRTemporary> result;
+ Optional<GPRReg> resultReg;
+ if (!resultIsUnboxed) {
+ result.emplace(this);
+ resultReg = result->gpr();
+ }
FPRTemporary temp(this);
- GPRReg resultReg = result.gpr();
FPRReg tempReg = temp.fpr();
MacroAssembler::JumpList slowCases;
@@ -2824,15 +2839,35 @@
slowCases.append(m_jit.branch32(MacroAssembler::AboveOrEqual, propertyReg, MacroAssembler::Address(storageReg, Butterfly::offsetOfPublicLength())));
m_jit.loadDouble(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesEight), tempReg);
- slowCases.append(m_jit.branchIfNaN(tempReg));
- boxDouble(tempReg, resultReg);
-
- addSlowPathGenerator(
- slowPathCall(
- slowCases, this, operationGetByValObjectInt,
- result.gpr(), TrustedImmPtr::weakPointer(m_graph, m_graph.globalObjectFor(node->origin.semantic)), baseReg, propertyReg));
-
- jsValueResult(resultReg, node);
+ if (node->arrayMode().isOutOfBoundsSaneChain()) {
+ if (resultIsUnboxed) {
+ auto done = m_jit.jump();
+ slowCases.link(&m_jit);
+ speculationCheck(NegativeIndex, JSValueRegs(), nullptr, m_jit.branch32(MacroAssembler::LessThan, propertyReg, CCallHelpers::TrustedImm32(0)));
+ static const double NaN = PNaN;
+ m_jit.loadDouble(TrustedImmPtr(&NaN), tempReg);
+ done.link(&m_jit);
+ doubleResult(tempReg, node);
+ } else {
+ slowCases.append(m_jit.branchIfNaN(tempReg));
+ boxDouble(tempReg, *resultReg);
+ auto done = m_jit.jump();
+ slowCases.link(&m_jit);
+ speculationCheck(NegativeIndex, JSValueRegs(), nullptr, m_jit.branch32(MacroAssembler::LessThan, propertyReg, CCallHelpers::TrustedImm32(0)));
+ m_jit.move(CCallHelpers::TrustedImm64(JSValue::encode(jsUndefined())), *resultReg);
+ done.link(&m_jit);
+ jsValueResult(*resultReg, node);
+ }
+ } else {
+ slowCases.append(m_jit.branchIfNaN(tempReg));
+ boxDouble(tempReg, *resultReg);
+ addSlowPathGenerator(
+ slowPathCall(
+ slowCases, this, operationGetByValObjectInt,
+ *resultReg, TrustedImmPtr::weakPointer(m_graph, m_graph.globalObjectFor(node->origin.semantic)), baseReg, propertyReg));
+ jsValueResult(*resultReg, node);
+ }
+
break;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGValidate.cpp (265774 => 265775)
--- trunk/Source/_javascript_Core/dfg/DFGValidate.cpp 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/dfg/DFGValidate.cpp 2020-08-17 22:10:14 UTC (rev 265775)
@@ -382,6 +382,20 @@
case NewArrayBuffer:
VALIDATE((node), node->vectorLengthHint() >= node->castOperand<JSImmutableButterfly*>()->length());
break;
+ case GetByVal:
+ switch (node->arrayMode().type()) {
+ case Array::Int32:
+ case Array::Double:
+ case Array::Contiguous:
+ // We rely on being an original array structure because we are SaneChain, and we need
+ // Array.prototype to be our prototype, so we can return undefined when we go OOB.
+ if (node->arrayMode().isOutOfBoundsSaneChain())
+ VALIDATE((node), node->arrayMode().isJSArrayWithOriginalStructure());
+ break;
+ default:
+ break;
+ }
+ break;
default:
break;
}
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (265774 => 265775)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-08-17 21:54:42 UTC (rev 265774)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-08-17 22:10:14 UTC (rev 265775)
@@ -4632,7 +4632,7 @@
if (m_node->arrayMode().isInBounds()) {
LValue result = m_out.load64(baseIndex(heap, storage, index, m_graph.varArgChild(m_node, 1)));
LValue isHole = m_out.isZero64(result);
- if (m_node->arrayMode().isSaneChain()) {
+ if (m_node->arrayMode().isInBoundsSaneChain()) {
DFG_ASSERT(
m_graph, m_node, m_node->arrayMode().type() == Array::Contiguous, m_node->arrayMode().type());
result = m_out.select(
@@ -4645,7 +4645,7 @@
setJSValue(result);
return;
}
-
+
LBasicBlock fastCase = m_out.newBlock();
LBasicBlock slowCase = m_out.newBlock();
LBasicBlock continuation = m_out.newBlock();
@@ -4663,7 +4663,12 @@
m_out.isZero64(fastResultValue), rarely(slowCase), usually(continuation));
m_out.appendTo(slowCase, continuation);
- ValueFromBlock slowResult = m_out.anchor(vmCall(Int64, operationGetByValObjectInt, weakPointer(globalObject), base, index));
+ ValueFromBlock slowResult;
+ if (m_node->arrayMode().isOutOfBoundsSaneChain()) {
+ speculate(NegativeIndex, noValue(), nullptr, m_out.lessThan(index, m_out.int32Zero));
+ slowResult = m_out.anchor(m_out.constInt64(JSValue::ValueUndefined));
+ } else
+ slowResult = m_out.anchor(vmCall(Int64, operationGetByValObjectInt, weakPointer(globalObject), base, index));
m_out.jump(continuation);
m_out.appendTo(continuation, lastNext);
@@ -4685,7 +4690,7 @@
LValue result = m_out.loadDouble(
baseIndex(heap, storage, index, m_graph.varArgChild(m_node, 1)));
- if (!m_node->arrayMode().isSaneChain()) {
+ if (!m_node->arrayMode().isInBoundsSaneChain()) {
speculate(
LoadFromHole, noValue(), nullptr,
m_out.doubleNotEqualOrUnordered(result, result));
@@ -4694,6 +4699,8 @@
break;
}
+ bool resultIsUnboxed = m_node->arrayMode().isOutOfBoundsSaneChain() && !(m_node->flags() & NodeBytecodeUsesAsOther);
+
LBasicBlock inBounds = m_out.newBlock();
LBasicBlock boxPath = m_out.newBlock();
LBasicBlock slowCase = m_out.newBlock();
@@ -4712,15 +4719,26 @@
rarely(slowCase), usually(boxPath));
m_out.appendTo(boxPath, slowCase);
- ValueFromBlock fastResult = m_out.anchor(boxDouble(doubleValue));
+ ValueFromBlock fastResult = m_out.anchor(resultIsUnboxed ? doubleValue : boxDouble(doubleValue));
m_out.jump(continuation);
m_out.appendTo(slowCase, continuation);
- ValueFromBlock slowResult = m_out.anchor(vmCall(Int64, operationGetByValObjectInt, weakPointer(globalObject), base, index));
+ ValueFromBlock slowResult;
+ if (m_node->arrayMode().isOutOfBoundsSaneChain()) {
+ speculate(NegativeIndex, noValue(), nullptr, m_out.lessThan(index, m_out.int32Zero));
+ if (resultIsUnboxed)
+ slowResult = m_out.anchor(m_out.constDouble(PNaN));
+ else
+ slowResult = m_out.anchor(m_out.constInt64(JSValue::encode(jsUndefined())));
+ } else
+ slowResult = m_out.anchor(vmCall(Int64, operationGetByValObjectInt, weakPointer(globalObject), base, index));
m_out.jump(continuation);
m_out.appendTo(continuation, lastNext);
- setJSValue(m_out.phi(Int64, fastResult, slowResult));
+ if (resultIsUnboxed)
+ setDouble(m_out.phi(Double, fastResult, slowResult));
+ else
+ setJSValue(m_out.phi(Int64, fastResult, slowResult));
return;
}
@@ -8135,7 +8153,7 @@
if (globalObject->stringPrototypeChainIsSane()) {
// FIXME: This could be captured using a Speculation mode that means
// "out-of-bounds loads return a trivial value", something like
- // SaneChainOutOfBounds.
+ // OutOfBoundsSaneChain.
// https://bugs.webkit.org/show_bug.cgi?id=144668
m_graph.registerAndWatchStructureTransition(stringPrototypeStructure);
@@ -12220,7 +12238,7 @@
LValue checkHoleResultValue =
m_out.notZero64(m_out.load64(baseIndex(heap, storage, index, m_graph.varArgChild(m_node, 1))));
ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
- if (mode.isSaneChain())
+ if (mode.isInBoundsSaneChain())
m_out.jump(continuation);
else if (!mode.isInBounds())
m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
@@ -12259,7 +12277,7 @@
LValue doubleValue = m_out.loadDouble(baseIndex(heap, storage, index, m_graph.varArgChild(m_node, 1)));
LValue checkHoleResultValue = m_out.doubleEqual(doubleValue, doubleValue);
ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
- if (mode.isSaneChain())
+ if (mode.isInBoundsSaneChain())
m_out.jump(continuation);
else if (!mode.isInBounds())
m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
@@ -12297,7 +12315,7 @@
LValue checkHoleResultValue =
m_out.notZero64(m_out.load64(baseIndex(m_heaps.ArrayStorage_vector, storage, index, m_graph.varArgChild(m_node, 1))));
ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
- if (mode.isSaneChain())
+ if (mode.isInBoundsSaneChain())
m_out.jump(continuation);
else if (!mode.isInBounds())
m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
