Attachment 339509 Details for Bug 185287 – work in progress

[patch] work in progress

blah.patch (text/plain), 13.23 KB, created by Filip Pizlo on 2018-05-03 19:43:30 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2018-05-03 19:43:30 PDT

Size: 13.23 KB

patch

obsolete

>Index: Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h	(revision 231337)
>+++ Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h	(working copy)
>@@ -149,13 +149,13 @@ public:
>     PhiChildren* phiChildren() { return m_phiChildren.get(); }
>     
> private:
>-    void clobberWorld(const CodeOrigin&, unsigned indexInBlock);
>+    void clobberWorld();
>     void didFoldClobberWorld();
>     
>     template<typename Functor>
>     void forAllValues(unsigned indexInBlock, Functor&);
>     
>-    void clobberStructures(unsigned indexInBlock);
>+    void clobberStructures();
>     void didFoldClobberStructures();
>     
>     void observeTransition(unsigned indexInBlock, RegisteredStructure from, RegisteredStructure to);
>Index: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h	(revision 231337)
>+++ Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h	(working copy)
>@@ -2280,23 +2280,21 @@ bool AbstractInterpreter<AbstractStateTy
>         break;
>             
>     case ToThis: {
>-        AbstractValue& source = forNode(node->child1());
>-        AbstractValue& destination = forNode(node);
>         bool strictMode = m_graph.executableFor(node->origin.semantic)->isStrictMode();
> 
>-        ToThisResult result = isToThisAnIdentity(m_vm, strictMode, source);
>+        ToThisResult result = isToThisAnIdentity(m_vm, strictMode, forNode(node->child1()));
>         if (result != ToThisResult::Dynamic) {
>             switch (result) {
>             case ToThisResult::Identity:
>                 m_state.setFoundConstants(true);
>-                destination = source;
>+                forNode(node) = forNode(node->child1());
>                 break;
>             case ToThisResult::Undefined:
>                 setConstant(node, jsUndefined());
>                 break;
>             case ToThisResult::GlobalThis:
>                 m_state.setFoundConstants(true);
>-                destination.setType(m_graph, SpecObject);
>+                forNode(node).setType(m_graph, SpecObject);
>                 break;
>             case ToThisResult::Dynamic:
>                 RELEASE_ASSERT_NOT_REACHED();
>@@ -2305,10 +2303,10 @@ bool AbstractInterpreter<AbstractStateTy
>         }
> 
>         if (strictMode)
>-            destination.makeHeapTop();
>+            forNode(node).makeHeapTop();
>         else {
>-            destination = source;
>-            destination.merge(SpecObject);
>+            forNode(node) = forNode(node->child1());
>+            forNode(node).merge(SpecObject);
>         }
>         break;
>     }
>@@ -2344,14 +2342,11 @@ bool AbstractInterpreter<AbstractStateTy
> 
>     case ToObject:
>     case CallObjectConstructor: {
>-        AbstractValue& source = forNode(node->child1());
>-        AbstractValue& destination = forNode(node);
>-
>-        if (!(source.m_type & ~SpecObject)) {
>+        if (!(forNode(node->child1()).m_type & ~SpecObject)) {
>             m_state.setFoundConstants(true);
>             if (node->op() == ToObject)
>                 didFoldClobberWorld();
>-            destination = source;
>+            forNode(node) = forNode(node->child1());
>             break;
>         }
> 
>@@ -2856,8 +2851,7 @@ bool AbstractInterpreter<AbstractStateTy
>         break;
>     }
>     case ArrayifyToStructure: {
>-        AbstractValue& value = forNode(node->child1());
>-        if (value.m_structure.isSubsetOf(RegisteredStructureSet(node->structure())))
>+        if (forNode(node->child1()).m_structure.isSubsetOf(RegisteredStructureSet(node->structure())))
>             m_state.setFoundConstants(true);
>         clobberStructures(clobberLimit);
>         
>@@ -2888,7 +2882,7 @@ bool AbstractInterpreter<AbstractStateTy
>         // Note that it's tempting to simply say that the resulting value is BOTTOM because of
>         // the contradiction. That would be wrong, since we haven't hit an invalidation point,
>         // yet.
>-        value.set(m_graph, node->structure());
>+        forNode(node->child1()).set(m_graph, node->structure());
>         break;
>     }
>     case GetIndexedPropertyStorage: {
>@@ -3400,7 +3394,6 @@ bool AbstractInterpreter<AbstractStateTy
>         break;
>         
>     case InvalidationPoint:
>-        forAllValues(clobberLimit, AbstractValue::observeInvalidationPointFor);
>         m_state.setStructureClobberState(StructuresAreWatched);
>         break;
> 
>@@ -3537,10 +3530,9 @@ bool AbstractInterpreter<AbstractStateTy
> }
> 
> template<typename AbstractStateType>
>-void AbstractInterpreter<AbstractStateType>::clobberWorld(
>-    const CodeOrigin&, unsigned clobberLimit)
>+void AbstractInterpreter<AbstractStateType>::clobberWorld()
> {
>-    clobberStructures(clobberLimit);
>+    clobberStructures();
> }
> 
> template<typename AbstractStateType>
>@@ -3579,9 +3571,9 @@ void AbstractInterpreter<AbstractStateTy
> }
> 
> template<typename AbstractStateType>
>-void AbstractInterpreter<AbstractStateType>::clobberStructures(unsigned clobberLimit)
>+void AbstractInterpreter<AbstractStateType>::clobberStructures()
> {
>-    forAllValues(clobberLimit, AbstractValue::clobberStructuresFor);
>+    m_state.clobberStructures();
>     m_state.mergeClobberState(AbstractInterpreterClobberState::ClobberedStructures);
>     m_state.setStructureClobberState(StructuresAreClobbered);
> }
>Index: Source/JavaScriptCore/dfg/DFGAbstractValue.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGAbstractValue.h	(revision 231337)
>+++ Source/JavaScriptCore/dfg/DFGAbstractValue.h	(working copy)
>@@ -103,22 +103,16 @@ struct AbstractValue {
>         checkConsistency();
>     }
>     
>-    static void clobberStructuresFor(AbstractValue& value)
>+    ALWAYS_INLINE void fastForwardTo(Epoch epoch, StructureClobberState clobberState)
>     {
>-        value.clobberStructures();
>-    }
>-    
>-    void observeInvalidationPoint()
>-    {
>-        m_structure.observeInvalidationPoint();
>+        if (epoch != m_effectEpoch)
>+            clobberStructures();
>+        if (clobberState == StructuresAreWatched)
>+            m_structure.observeInvalidationPoint();
>+        m_effectEpoch = epoch;
>         checkConsistency();
>     }
>     
>-    static void observeInvalidationPointFor(AbstractValue& value)
>-    {
>-        value.observeInvalidationPoint();
>-    }
>-    
>     void observeTransition(RegisteredStructure from, RegisteredStructure to)
>     {
>         if (m_type & SpecCell) {
>@@ -397,6 +391,10 @@ struct AbstractValue {
>     // effect that makes non-obvious changes to the heap.
>     ArrayModes m_arrayModes;
>     
>+    // The epoch.
>+    // FIXME: Explain this. (OOPS!!)
>+    Epoch m_effectEpoch;
>+    
>     // This is a proven constraint on the possible values that this value can
>     // have now or any time in the future, unless it is reassigned. Note that this
>     // implies nothing about the structure. Oddly, JSValue() (i.e. the empty value)
>Index: Source/JavaScriptCore/dfg/DFGAtTailAbstractState.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGAtTailAbstractState.h	(revision 231337)
>+++ Source/JavaScriptCore/dfg/DFGAtTailAbstractState.h	(working copy)
>@@ -52,7 +52,18 @@ public:
>     void createValueForNode(NodeFlowProjection);
>     AbstractValue& forNode(NodeFlowProjection);
>     AbstractValue& forNode(Edge edge) { return forNode(edge.node()); }
>-    Operands<AbstractValue>& variables() { return m_block->valuesAtTail; }
>+    
>+    unsigned numberOfArguments() const { return m_block->valuesAtTail.numberOfArguments(); }
>+    unsigend numberOfLocals() const { return m_block->valuesAtTail.numberOfLocals(); }
>+    AbstractValue& operand(int operand) { return m_block->valuesAtTail.operand(operand); }
>+    AbstractValue& operand(VirtualRegister operand) { return m_block->valuesAtTail.operand(operand); }
>+    AbstractValue& local(size_t index) { return m_block->valuesAtTail.local(index); }
>+    AbstractValue& argument(size_t index) { return m_block->valuesAtTail.argument(index); }
>+    
>+    void clobberStructures()
>+    {
>+        UNREACHABLE_FOR_PLATFORM();
>+    }
>     
>     BasicBlock* block() const { return m_block; }
>     
>Index: Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp	(revision 231337)
>+++ Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2013-2016 Apple Inc. All rights reserved.
>+ * Copyright (C) 2013-2018 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -60,6 +60,7 @@ void InPlaceAbstractState::beginBasicBlo
>     ASSERT(basicBlock->variablesAtHead.numberOfLocals() == basicBlock->variablesAtTail.numberOfLocals());
> 
>     m_abstractValues.resize();
>+    m_effectEpoch = Epoch::first();
>     
>     for (size_t i = 0; i < basicBlock->size(); i++) {
>         NodeFlowProjection::forEach(
>@@ -68,7 +69,8 @@ void InPlaceAbstractState::beginBasicBlo
>             });
>     }
> 
>-    m_variables = basicBlock->valuesAtHead;
>+    for (size_t i = m_variables.size(); i--;)
>+        variableAt(i) = basicBlock->valuesAtHead[i];
>     
>     if (m_graph.m_form == SSA) {
>         for (NodeAbstractValuePair& entry : basicBlock->ssa->valuesAtHead) {
>@@ -201,19 +203,19 @@ bool InPlaceAbstractState::endBasicBlock
>     case ThreadedCPS: {
>         for (size_t argument = 0; argument < block->variablesAtTail.numberOfArguments(); ++argument) {
>             AbstractValue& destination = block->valuesAtTail.argument(argument);
>-            mergeStateAtTail(destination, m_variables.argument(argument), block->variablesAtTail.argument(argument));
>+            mergeStateAtTail(destination, argument(argument), block->variablesAtTail.argument(argument));
>         }
> 
>         for (size_t local = 0; local < block->variablesAtTail.numberOfLocals(); ++local) {
>             AbstractValue& destination = block->valuesAtTail.local(local);
>-            mergeStateAtTail(destination, m_variables.local(local), block->variablesAtTail.local(local));
>+            mergeStateAtTail(destination, local(local), block->variablesAtTail.local(local));
>         }
>         break;
>     }
> 
>     case SSA: {
>         for (size_t i = 0; i < block->valuesAtTail.size(); ++i)
>-            block->valuesAtTail[i].merge(m_variables[i]);
>+            block->valuesAtTail[i].merge(variableAt(i));
> 
>         for (NodeAbstractValuePair& valueAtTail : block->ssa->valuesAtTail) {
>             AbstractValue& valueAtNode = forNode(valueAtTail.node);
>Index: Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.h	(revision 231337)
>+++ Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.h	(working copy)
>@@ -49,7 +49,7 @@ public:
>     
>     AbstractValue& forNode(NodeFlowProjection node)
>     {
>-        return m_abstractValues.at(node);
>+        return fastForward(m_abstractValues.at(node));
>     }
>     
>     AbstractValue& forNode(Edge edge)
>@@ -57,10 +57,13 @@ public:
>         return forNode(edge.node());
>     }
>     
>-    Operands<AbstractValue>& variables()
>-    {
>-        return m_variables;
>-    }
>+    unsigned numberOfArguments() const { return m_variables.numberOfArguments(); }
>+    unsigend numberOfLocals() const { return m_variables.numberOfLocals(); }
>+    AbstractValue& operand(int operand) { return fastForward(m_variables.operand(operand)); }
>+    AbstractValue& operand(VirtualRegister operand) { return fastForward(m_variables.operand(operand)); }
>+    AbstractValue& local(size_t index) { return fastForward(m_variables.local(index)); }
>+    AbstractValue& argument(size_t index) { return fastForward(m_variables.argument(index)); }
>+    AbstractValue& variableAt(size_t index) { return fastForward(m_variables[index]); }
>     
>     // Call this before beginning CFA to initialize the abstract values of
>     // arguments, and to indicate which blocks should be listed for CFA
>@@ -119,6 +122,8 @@ public:
>     // MergeToSuccessors.
>     bool mergeToSuccessors(BasicBlock*);
>     
>+    void clobberStructures() { m_effectEpoch.bump(); }
>+    
>     // Methods intended to be called from AbstractInterpreter.
>     void setClobberState(AbstractInterpreterClobberState state) { m_clobberState = state; }
>     void mergeClobberState(AbstractInterpreterClobberState state) { m_clobberState = mergeClobberStates(m_clobberState, state); }
>@@ -139,6 +144,12 @@ public:
>     }
> 
> private:
>+    AbstractValue& fastForward(AbstractValue& value)
>+    {
>+        value.fastForwardTo(m_effectEpoch, m_structureClobberState);
>+        return value;
>+    }
>+    
>     void mergeStateAtTail(AbstractValue& destination, AbstractValue& inVariable, Node*);
> 
>     static bool mergeVariableBetweenBlocks(AbstractValue& destination, AbstractValue& source, Node* destinationNode, Node* sourceNode);
>@@ -154,6 +165,7 @@ private:
>     bool m_isValid;
>     AbstractInterpreterClobberState m_clobberState;
>     StructureClobberState m_structureClobberState;
>+    Epoch m_effectEpoch;
>     
>     BranchDirection m_branchDirection; // This is only set for blocks that end in Branch and that execute to completion (i.e. m_isValid == true).
> };

Actions: View | Formatted Diff | Diff

Attachments on bug 185287: 339509 | 339620 | 339623 | 339650 | 339661 | 339662 | 339663 | 339687