Changeset 253991 in webkit for trunk/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp

Timestamp:

Jan 3, 2020, 1:20:48 AM (5 years ago)

Author:

[email protected]

Message:

AI rule for PutById can only observe transitions when it watches the condition
​https://bugs.webkit.org/show_bug.cgi?id=205697
<rdar://problem/56814254>

Reviewed by Yusuke Suzuki.

JSTests:

• stress/only-transition-structures-for-put-by-id-in-AI-if-watchable.js: Added.

Source/JavaScriptCore:

There was a bug in AI where we were capturing a PutByIdStatus and
emitting a structure transition in AI state based on the variants inside this
PutByIdStatus. This, in principal, is a valid static analysis to perform.
However, we can only do this if we ensure that the snapshot we have in the
PutByIdStatus holds at runtime. We can do this by watching the property conditions
for the various variants. AI forgot to watch these conditions. This patch fixes that.
In practice, this also means we need to be slightly more strict about stating to
AI when we transition since some object property conditions aren't watchable, and need
to be verified at runtime via structure checks. This is ok in practice, since
we'll emit the code to do that inside constant folding (constant folding was
already doing this), which will continue to report the precise transition in
the abstract state.

• dfg/DFGAbstractInterpreter.h:
• dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

• dfg/DFGConstantFoldingPhase.cpp:

(JSC::DFG::ConstantFoldingPhase::foldConstants):

File:

• trunk/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp (modified) (4 diffs)

trunk/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp

@@ -607 +607 @@
                 AbstractValue valueValue = m_state.forNode(node->child2());
 
-                m_interpreter.execute(indexInBlock); // Push CFA over this node after we get the state before.
-                alreadyHandled = true; // Don't allow the default constant folder to do things to this.
-
                 if (!baseValue.m_structure.isFinite())
                     break;
-                
+
                 PutByIdStatus status = PutByIdStatus::computeFor(
                     m_graph.globalObjectFor(origin.semantic),
@@ -618 +615 @@
                     m_graph.identifiers()[identifierNumber],
                     node->op() == PutByIdDirect);
-                
+
                 if (!status.isSimple())
                     break;
@@ -630 +627 @@
 
                 bool allGood = true;
+                RegisteredStructureSet newSet;
+                TransitionVector transitions;
                 for (const PutByIdVariant& variant : status.variants()) {
-                    if (!allGood)
-                        break;
                     for (const ObjectPropertyCondition& condition : variant.conditionSet()) {
                         if (m_graph.watchCondition(condition))
@@ -649 +646 @@
                                 indexInBlock, node->origin, condition.object(), KnownCellUse));
                     }
+
+                    if (!allGood)
+                        break;
+
+                    if (variant.kind() == PutByIdVariant::Transition) {
+                        RegisteredStructure newStructure = m_graph.registerStructure(variant.newStructure());
+                        transitions.append(
+                            Transition(
+                                m_graph.registerStructure(variant.oldStructureForTransition()), newStructure));
+                        newSet.add(newStructure);
+                    } else {
+                        ASSERT(variant.kind() == PutByIdVariant::Replace);
+                        newSet.merge(*m_graph.addStructureSet(variant.oldStructure()));
+                    }
                 }
 
                 if (!allGood)
                     break;
+
+                // Push CFA over this node after we get the state before.
+                m_interpreter.didFoldClobberWorld();
+                m_interpreter.observeTransitions(indexInBlock, transitions);
+                if (m_state.forNode(node->child1()).changeStructure(m_graph, newSet) == Contradiction)
+                    m_state.setIsValid(false);
+
+                alreadyHandled = true; // Don't allow the default constant folder to do things to this.
                 
                 m_insertionSet.insertNode(