Sane chain and string watchpoints should be set in FixupPhase or the backend rather than WatchpointCollectionPhase
https://bugs.webkit.org/show_bug.cgi?id=144665
Reviewed by Michael Saboff.
This is a step towards getting rid of WatchpointCollectionPhase. It's also a step towards
extending SaneChain to all indexing shapes.
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode): Set the watchpoints here so that we don't need a case in WatchpointCollectionPhase.
(JSC::DFG::FixupPhase::checkArray): Clarify the need for checking the structure. We often forget why we do this instead of always using CheckArray.
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileGetByValOnString): Set the watchpoints here so that we don't need a case in WatchpointCollectionPhase.
* dfg/DFGWatchpointCollectionPhase.cpp:
(JSC::DFG::WatchpointCollectionPhase::handle): Remove some code.
(JSC::DFG::WatchpointCollectionPhase::handleStringGetByVal): Deleted.
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::compileStringCharAt): Set the watchpoints here so that we don't need a case in WatchpointCollectionPhase.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@183897 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index 8be0089..7ab1b16 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -28,6 +28,7 @@
#if ENABLE(DFG_JIT)
+#include "ArrayPrototype.h"
#include "DFGGraph.h"
#include "DFGInsertionSet.h"
#include "DFGPhase.h"
@@ -517,10 +518,17 @@
switch (arrayMode.type()) {
case Array::Double:
if (arrayMode.arrayClass() == Array::OriginalArray
- && arrayMode.speculation() == Array::InBounds
- && m_graph.globalObjectFor(node->origin.semantic)->arrayPrototypeChainIsSane()
- && !(node->flags() & NodeBytecodeUsesAsOther))
- node->setArrayMode(arrayMode.withSpeculation(Array::SaneChain));
+ && arrayMode.speculation() == Array::InBounds) {
+ JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
+ if (globalObject->arrayPrototypeChainIsSane()
+ && !(node->flags() & NodeBytecodeUsesAsOther)) {
+ m_graph.watchpoints().addLazily(
+ globalObject->arrayPrototype()->structure()->transitionWatchpointSet());
+ m_graph.watchpoints().addLazily(
+ globalObject->objectPrototype()->structure()->transitionWatchpointSet());
+ node->setArrayMode(arrayMode.withSpeculation(Array::SaneChain));
+ }
+ }
break;
case Array::String:
@@ -1542,10 +1550,12 @@
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
+ // that needs to protect against JSArray's __proto__ being changed.
Structure* structure = arrayMode.originalArrayStructure(m_graph, origin.semantic);
Edge indexEdge = index ? Edge(index, Int32Use) : Edge();
-
+
if (arrayMode.doesConversion()) {
if (structure) {
m_insertionSet.insertNode(
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
index e2a2369..36d9bf0 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -1756,6 +1756,15 @@
JSGlobalObject* globalObject = m_jit.globalObjectFor(node->origin.semantic);
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
+ // 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.
+ // https://bugs.webkit.org/show_bug.cgi?id=144668
+ m_jit.graph().watchpoints().addLazily(globalObject->stringPrototype()->structure()->transitionWatchpointSet());
+ m_jit.graph().watchpoints().addLazily(globalObject->objectPrototype()->structure()->transitionWatchpointSet());
+
#if USE(JSVALUE64)
addSlowPathGenerator(std::make_unique<SaneStringGetByValSlowPathGenerator>(
outOfBounds, this, JSValueRegs(scratchReg), baseReg, propertyReg));
diff --git a/Source/JavaScriptCore/dfg/DFGWatchpointCollectionPhase.cpp b/Source/JavaScriptCore/dfg/DFGWatchpointCollectionPhase.cpp
index ee1f6cf..8412fae 100644
--- a/Source/JavaScriptCore/dfg/DFGWatchpointCollectionPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGWatchpointCollectionPhase.cpp
@@ -34,6 +34,14 @@
#include "DFGPhase.h"
#include "JSCInlines.h"
+// FIXME: Remove this phase entirely by moving the addLazily() calls into either the backend or
+// into the phase that performs the optimization. Moving the calls into the backend makes the most
+// sense when the intermediate phases don't need to know that the watchpoint was set. Moving the
+// calls earlier usually only makes sense if the node's only purpose was to convey the need for
+// the watchpoint (like VarInjectionWatchpoint). But, it can also make sense if the fact that the
+// watchpoint was set enables other optimizations.
+// https://bugs.webkit.org/show_bug.cgi?id=144669
+
namespace JSC { namespace DFG {
class WatchpointCollectionPhase : public Phase {
@@ -83,21 +91,6 @@
handleMasqueradesAsUndefined();
break;
- case GetByVal:
- if (m_node->arrayMode().type() == Array::Double
- && m_node->arrayMode().isSaneChain()) {
- addLazily(globalObject()->arrayPrototype()->structure()->transitionWatchpointSet());
- addLazily(globalObject()->objectPrototype()->structure()->transitionWatchpointSet());
- }
-
- if (m_node->arrayMode().type() == Array::String)
- handleStringGetByVal();
- break;
-
- case StringCharAt:
- handleStringGetByVal();
- break;
-
case NewArray:
case NewArrayWithSize:
case NewArrayBuffer:
@@ -120,16 +113,6 @@
addLazily(globalObject()->masqueradesAsUndefinedWatchpoint());
}
- void handleStringGetByVal()
- {
- if (!m_node->arrayMode().isOutOfBounds())
- return;
- if (!globalObject()->stringPrototypeChainIsSane())
- return;
- addLazily(globalObject()->stringPrototype()->structure()->transitionWatchpointSet());
- addLazily(globalObject()->objectPrototype()->structure()->transitionWatchpointSet());
- }
-
void addLazily(WatchpointSet* set)
{
m_graph.watchpoints().addLazily(set);
