Attachment 339929 Details for Bug 185457 – more

[patch] more

blah.patch (text/plain), 19.26 KB, created by Filip Pizlo on 2018-05-08 22:16:40 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2018-05-08 22:16:40 PDT

Size: 19.26 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 231523)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+2018-05-08  Filip Pizlo  <fpizlo@apple.com>
>+
>+        AbstractValue::filter should have an inline fast path
>+        https://bugs.webkit.org/show_bug.cgi?id=185457
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        This adds a fast path for the most common case of filtering.
>+
>+        * dfg/DFGAbstractValue.cpp:
>+        (JSC::DFG::AbstractValue::filterSlow):
>+        * dfg/DFGAbstractValue.h:
>+        (JSC::DFG::AbstractValue::filter):
>+
> 2018-05-08  Filip Pizlo  <fpizlo@apple.com>
> 
>         DFG::FlowMap::resize() shouldn't resize the shadow map unless we're in SSA
>Index: Source/JavaScriptCore/bytecode/ExitKind.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecode/ExitKind.cpp	(revision 231522)
>+++ Source/JavaScriptCore/bytecode/ExitKind.cpp	(working copy)
>@@ -104,9 +104,6 @@ bool exitKindMayJettison(ExitKind kind)
>     default:
>         return true;
>     }
>-
>-    RELEASE_ASSERT_NOT_REACHED();
>-    return false;
> }
> 
> } // namespace JSC
>Index: Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h	(revision 231522)
>+++ Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h	(working copy)
>@@ -160,7 +160,10 @@ public:
>     
>     ALWAYS_INLINE void filterEdgeByUse(Edge& edge)
>     {
>-        filterByType(edge, typeFilterFor(edge.useKind()));
>+        UseKind useKind = edge.useKind();
>+        if (useKind == UntypedUse)
>+            return;
>+        filterByType(edge, typeFilterFor(useKind));
>     }
>     
>     // Abstractly execute the effects of the given node. This changes the abstract
>@@ -242,12 +245,7 @@ private:
>         m_state.setFoundConstants(true);
>     }
>     
>-    ALWAYS_INLINE void filterByType(Edge& edge, SpeculatedType type)
>-    {
>-        AbstractValue& value = forNode(edge);
>-        m_state.setProofStatus(edge, value.isType(type) ? IsProved : NeedsCheck);
>-        filter(value, type);
>-    }
>+    ALWAYS_INLINE void filterByType(Edge& edge, SpeculatedType type);
>     
>     void verifyEdge(Node*, Edge);
>     void verifyEdges(Node*);
>Index: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h	(revision 231522)
>+++ Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h	(working copy)
>@@ -102,13 +102,27 @@ void AbstractInterpreter<AbstractStateTy
> }
> 
> template<typename AbstractStateType>
>+class AbstractInterpreterExecuteEdgesFunc {
>+public:
>+    AbstractInterpreterExecuteEdgesFunc(AbstractInterpreter<AbstractStateType>& interpreter)
>+        : m_interpreter(interpreter)
>+    {
>+    }
>+    
>+    // This func is manually written out so that we can put ALWAYS_INLINE on it.
>+    ALWAYS_INLINE void operator()(Edge& edge) const
>+    {
>+        m_interpreter.filterEdgeByUse(edge);
>+    }
>+    
>+private:
>+    AbstractInterpreter<AbstractStateType>& m_interpreter;
>+};
>+
>+template<typename AbstractStateType>
> void AbstractInterpreter<AbstractStateType>::executeEdges(Node* node)
> {
>-    m_graph.doToChildren(
>-        node,
>-        [&] (Edge& edge) {
>-            filterEdgeByUse(edge);
>-        });
>+    m_graph.doToChildren(node, AbstractInterpreterExecuteEdgesFunc<AbstractStateType>(*this));
> }
> 
> template<typename AbstractStateType>
>@@ -130,9 +144,21 @@ void AbstractInterpreter<AbstractStateTy
> }
> 
> template<typename AbstractStateType>
>+ALWAYS_INLINE void AbstractInterpreter<AbstractStateType>::filterByType(Edge& edge, SpeculatedType type)
>+{
>+    AbstractValue& value = m_state.forNodeWithoutFastForward(edge);
>+    if (value.isType(type)) {
>+        m_state.setProofStatus(edge, IsProved);
>+        return;
>+    }
>+    m_state.setProofStatus(edge, NeedsCheck);
>+    m_state.fastForwardAndFilterUnproven(value, type);
>+}
>+
>+template<typename AbstractStateType>
> void AbstractInterpreter<AbstractStateType>::verifyEdge(Node* node, Edge edge)
> {
>-    if (!(forNode(edge).m_type & ~typeFilterFor(edge.useKind())))
>+    if (!(m_state.forNodeWithoutFastForward(edge).m_type & ~typeFilterFor(edge.useKind())))
>         return;
>     
>     DFG_CRASH(m_graph, node, toCString("Edge verification error: ", node, "->", edge, " was expected to have type ", SpeculationDump(typeFilterFor(edge.useKind())), " but has type ", SpeculationDump(forNode(edge).m_type), " (", forNode(edge).m_type, ")").data(), AbstractInterpreterInvalidType, node->op(), edge->op(), edge.useKind(), forNode(edge).m_type);
>@@ -999,7 +1025,7 @@ bool AbstractInterpreter<AbstractStateTy
>         default:
>             DFG_ASSERT(m_graph, node, node->child1().useKind() == UntypedUse, node->child1().useKind());
>             clobberWorld();
>-            setNonCellTypeForNode(node, SpecFullNumber);
>+            setNonCellTypeForNode(node, SpecBytecodeNumber);
>             break;
>         }
>         break;
>@@ -1079,7 +1105,7 @@ bool AbstractInterpreter<AbstractStateTy
>         } else {
>             DFG_ASSERT(m_graph, node, node->child1().useKind() == UntypedUse, node->child1().useKind());
>             clobberWorld();
>-            setNonCellTypeForNode(node, SpecFullNumber);
>+            setNonCellTypeForNode(node, SpecBytecodeNumber);
>         }
>         break;
>     }
>@@ -3712,11 +3738,13 @@ void AbstractInterpreter<AbstractStateTy
>         setConstant(node, jsDoubleNumber(equivalentFunction(*number)));
>         return;
>     }
>-    SpeculatedType type = SpecFullNumber;
>+    SpeculatedType type;
>     if (node->child1().useKind() == DoubleRepUse)
>         type = typeOfDoubleUnaryOp(forNode(node->child1()).m_type);
>-    else
>+    else {
>         clobberWorld();
>+        type = SpecBytecodeNumber;
>+    }
>     setNonCellTypeForNode(node, type);
> }
> 
>Index: Source/JavaScriptCore/dfg/DFGAbstractValue.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGAbstractValue.cpp	(revision 231522)
>+++ Source/JavaScriptCore/dfg/DFGAbstractValue.cpp	(working copy)
>@@ -332,24 +332,8 @@ FiltrationResult AbstractValue::filterCl
>     return normalizeClarity(graph);
> }
> 
>-FiltrationResult AbstractValue::filter(SpeculatedType type)
>+FiltrationResult AbstractValue::filterSlow(SpeculatedType type)
> {
>-    if ((m_type & type) == m_type)
>-        return FiltrationOK;
>-    
>-    // Fast path for the case that we don't even have a cell.
>-    if (!(m_type & SpecCell)) {
>-        m_type &= type;
>-        FiltrationResult result;
>-        if (m_type == SpecNone) {
>-            clear();
>-            result = Contradiction;
>-        } else
>-            result = FiltrationOK;
>-        checkConsistency();
>-        return result;
>-    }
>-    
>     m_type &= type;
>     
>     // It's possible that prior to this filter() call we had, say, (Final, TOP), and
>@@ -362,6 +346,14 @@ FiltrationResult AbstractValue::filter(S
>     return normalizeClarity();
> }
> 
>+FiltrationResult AbstractValue::fastForwardToAndFilterSlow(AbstractValueClobberEpoch newEpoch, SpeculatedType type)
>+{
>+    if (newEpoch != m_effectEpoch)
>+        fastForwardToSlow(newEpoch);
>+    
>+    return filterSlow(type);
>+}
>+
> FiltrationResult AbstractValue::filterByValue(const FrozenValue& value)
> {
>     FiltrationResult result = filter(speculationFromValue(value.value()));
>Index: Source/JavaScriptCore/dfg/DFGAbstractValue.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGAbstractValue.h	(revision 231522)
>+++ Source/JavaScriptCore/dfg/DFGAbstractValue.h	(working copy)
>@@ -312,13 +312,59 @@ struct AbstractValue {
>     FiltrationResult filter(Graph&, const RegisteredStructureSet&, SpeculatedType admittedTypes = SpecNone);
>     
>     FiltrationResult filterArrayModes(ArrayModes);
>-    FiltrationResult filter(SpeculatedType);
>+
>+    ALWAYS_INLINE FiltrationResult filter(SpeculatedType type)
>+    {
>+        if ((m_type & type) == m_type)
>+            return FiltrationOK;
>+    
>+        // Fast path for the case that we don't even have a cell.
>+        if (!(m_type & SpecCell)) {
>+            m_type &= type;
>+            FiltrationResult result;
>+            if (m_type == SpecNone) {
>+                clear();
>+                result = Contradiction;
>+            } else
>+                result = FiltrationOK;
>+            checkConsistency();
>+            return result;
>+        }
>+        
>+        return filterSlow(type);
>+    }
>+    
>     FiltrationResult filterByValue(const FrozenValue& value);
>     FiltrationResult filter(const AbstractValue&);
>     FiltrationResult filterClassInfo(Graph&, const ClassInfo*);
> 
>     FiltrationResult filter(Graph&, const InferredType::Descriptor&);
>     
>+    ALWAYS_INLINE FiltrationResult fastForwardToAndFilter(AbstractValueClobberEpoch newEpoch, SpeculatedType type)
>+    {
>+        if (isType(type))
>+            return FiltrationOK;
>+        
>+        return fastForwardToAndFilterUnproven(newEpoch, type);
>+    }
>+    
>+    ALWAYS_INLINE FiltrationResult fastForwardToAndFilterUnproven(AbstractValueClobberEpoch newEpoch, SpeculatedType type)
>+    {
>+        if (m_type & SpecCell)
>+            return fastForwardToAndFilterSlow(newEpoch, type);
>+        
>+        m_effectEpoch = newEpoch;
>+        m_type &= type;
>+        FiltrationResult result;
>+        if (m_type == SpecNone) {
>+            clear();
>+            result = Contradiction;
>+        } else
>+            result = FiltrationOK;
>+        checkConsistency();
>+        return result;
>+    }
>+
>     FiltrationResult changeStructure(Graph&, const RegisteredStructureSet&);
>     
>     bool contains(RegisteredStructure) const;
>@@ -469,7 +515,7 @@ private:
>     
>     void makeTop(SpeculatedType top)
>     {
>-        m_type |= top;
>+        m_type = top;
>         m_arrayModes = ALL_ARRAY_MODES;
>         m_structure.makeTop();
>         m_value = JSValue();
>@@ -477,6 +523,8 @@ private:
>     }
>     
>     void fastForwardToSlow(AbstractValueClobberEpoch);
>+    FiltrationResult filterSlow(SpeculatedType);
>+    FiltrationResult fastForwardToAndFilterSlow(AbstractValueClobberEpoch, SpeculatedType);
>     
>     void filterValueByType();
>     void filterArrayModesByType();
>Index: Source/JavaScriptCore/dfg/DFGAtTailAbstractState.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGAtTailAbstractState.h	(revision 231522)
>+++ Source/JavaScriptCore/dfg/DFGAtTailAbstractState.h	(working copy)
>@@ -50,9 +50,27 @@ public:
>     }
>     
>     void createValueForNode(NodeFlowProjection);
>+    
>+    AbstractValue& fastForward(AbstractValue& value) { return value; }
>+    
>     AbstractValue& forNode(NodeFlowProjection);
>     AbstractValue& forNode(Edge edge) { return forNode(edge.node()); }
>     
>+    ALWAYS_INLINE AbstractValue& forNodeWithoutFastForward(NodeFlowProjection node)
>+    {
>+        return forNode(node);
>+    }
>+    
>+    ALWAYS_INLINE AbstractValue& forNodeWithoutFastForward(Edge edge)
>+    {
>+        return forNode(edge);
>+    }
>+    
>+    ALWAYS_INLINE void fastForwardAndFilterUnproven(AbstractValue& value, SpeculatedType type)
>+    {
>+        value.filter(type);
>+    }
>+    
>     ALWAYS_INLINE void clearForNode(NodeFlowProjection node)
>     {
>         forNode(node).clear();
>Index: Source/JavaScriptCore/dfg/DFGGraph.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGGraph.h	(revision 231522)
>+++ Source/JavaScriptCore/dfg/DFGGraph.h	(working copy)
>@@ -716,11 +716,24 @@ public:
>     template<typename ChildFunctor>
>     ALWAYS_INLINE void doToChildren(Node* node, const ChildFunctor& functor)
>     {
>-        doToChildrenWithNode(
>-            node,
>-            [&functor] (Node*, Edge& edge) {
>-                functor(edge);
>-            });
>+        class ForwardingFunc {
>+        public:
>+            ForwardingFunc(const ChildFunctor& functor)
>+                : m_functor(functor)
>+            {
>+            }
>+            
>+            // This is a manually written func because we want ALWAYS_INLINE.
>+            ALWAYS_INLINE void operator()(Node*, Edge& edge) const
>+            {
>+                m_functor(edge);
>+            }
>+        
>+        private:
>+            const ChildFunctor& m_functor;
>+        };
>+    
>+        doToChildrenWithNode(node, ForwardingFunc(functor));
>     }
>     
>     bool uses(Node* node, Node* child)
>Index: Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.h	(revision 231522)
>+++ Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.h	(working copy)
>@@ -47,6 +47,27 @@ public:
>     
>     void createValueForNode(NodeFlowProjection) { }
>     
>+    ALWAYS_INLINE AbstractValue& fastForward(AbstractValue& value)
>+    {
>+        value.fastForwardTo(m_effectEpoch);
>+        return value;
>+    }
>+    
>+    ALWAYS_INLINE void fastForwardAndFilterUnproven(AbstractValue& value, SpeculatedType type)
>+    {
>+        value.fastForwardToAndFilterUnproven(m_effectEpoch, type);
>+    }
>+    
>+    ALWAYS_INLINE AbstractValue& forNodeWithoutFastForward(NodeFlowProjection node)
>+    {
>+        return m_abstractValues.at(node);
>+    }
>+    
>+    ALWAYS_INLINE AbstractValue& forNodeWithoutFastForward(Edge edge)
>+    {
>+        return forNodeWithoutFastForward(edge.node());
>+    }
>+    
>     ALWAYS_INLINE AbstractValue& forNode(NodeFlowProjection node)
>     {
>         return fastForward(m_abstractValues.at(node));
>@@ -226,12 +247,6 @@ public:
>     }
> 
> private:
>-    ALWAYS_INLINE AbstractValue& fastForward(AbstractValue& value)
>-    {
>-        value.fastForwardTo(m_effectEpoch);
>-        return value;
>-    }
>-    
>     void mergeStateAtTail(AbstractValue& destination, AbstractValue& inVariable, Node*);
> 
>     static bool mergeVariableBetweenBlocks(AbstractValue& destination, AbstractValue& source, Node* destinationNode, Node* sourceNode);
>Index: Source/JavaScriptCore/dfg/DFGOSRExit.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOSRExit.cpp	(revision 231522)
>+++ Source/JavaScriptCore/dfg/DFGOSRExit.cpp	(working copy)
>@@ -716,7 +716,7 @@ void OSRExit::executeOSRExit(Context& co
>     // counterValueForOptimizeAfterWarmUp().
> 
>     if (UNLIKELY(codeBlock->updateOSRExitCounterAndCheckIfNeedToReoptimize(exitState) == CodeBlock::OptimizeAction::ReoptimizeNow))
>-        triggerReoptimizationNow(baselineCodeBlock, &exit);
>+        triggerReoptimizationNow(baselineCodeBlock, codeBlock, &exit);
> 
>     reifyInlinedCallFrames(context, baselineCodeBlock, exit);
>     adjustAndJumpToTarget(context, vm, codeBlock, baselineCodeBlock, exit);
>Index: Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp	(revision 231522)
>+++ Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp	(working copy)
>@@ -49,13 +49,13 @@ void handleExitCounts(CCallHelpers& jit,
> 
>     jit.add32(AssemblyHelpers::TrustedImm32(1), AssemblyHelpers::AbsoluteAddress(&exit.m_count));
>     
>-    jit.move(AssemblyHelpers::TrustedImmPtr(jit.codeBlock()), GPRInfo::regT0);
>+    jit.move(AssemblyHelpers::TrustedImmPtr(jit.codeBlock()), GPRInfo::regT3);
>     
>     AssemblyHelpers::Jump tooFewFails;
>     
>-    jit.load32(AssemblyHelpers::Address(GPRInfo::regT0, CodeBlock::offsetOfOSRExitCounter()), GPRInfo::regT2);
>+    jit.load32(AssemblyHelpers::Address(GPRInfo::regT3, CodeBlock::offsetOfOSRExitCounter()), GPRInfo::regT2);
>     jit.add32(AssemblyHelpers::TrustedImm32(1), GPRInfo::regT2);
>-    jit.store32(GPRInfo::regT2, AssemblyHelpers::Address(GPRInfo::regT0, CodeBlock::offsetOfOSRExitCounter()));
>+    jit.store32(GPRInfo::regT2, AssemblyHelpers::Address(GPRInfo::regT3, CodeBlock::offsetOfOSRExitCounter()));
>     
>     jit.move(AssemblyHelpers::TrustedImmPtr(jit.baselineCodeBlock()), GPRInfo::regT0);
>     AssemblyHelpers::Jump reoptimizeNow = jit.branch32(
>@@ -101,14 +101,7 @@ void handleExitCounts(CCallHelpers& jit,
>     
>     reoptimizeNow.link(&jit);
>     
>-    // Reoptimize as soon as possible.
>-#if !NUMBER_OF_ARGUMENT_REGISTERS
>-    jit.poke(GPRInfo::regT0);
>-    jit.poke(AssemblyHelpers::TrustedImmPtr(&exit), 1);
>-#else
>-    jit.move(GPRInfo::regT0, GPRInfo::argumentGPR0);
>-    jit.move(AssemblyHelpers::TrustedImmPtr(&exit), GPRInfo::argumentGPR1);
>-#endif
>+    jit.setupArguments<decltype(triggerReoptimizationNow)>(GPRInfo::regT0, GPRInfo::regT3, AssemblyHelpers::TrustedImmPtr(&exit));
>     jit.move(AssemblyHelpers::TrustedImmPtr(tagCFunctionPtr<OperationPtrTag>(triggerReoptimizationNow)), GPRInfo::nonArgGPR0);
>     jit.call(GPRInfo::nonArgGPR0, OperationPtrTag);
>     AssemblyHelpers::Jump doneAdjusting = jit.jump();
>Index: Source/JavaScriptCore/dfg/DFGOperations.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOperations.cpp	(revision 231522)
>+++ Source/JavaScriptCore/dfg/DFGOperations.cpp	(working copy)
>@@ -2815,7 +2815,7 @@ void JIT_OPERATION operationThrowStaticE
>     scope.throwException(exec, createError(exec, static_cast<ErrorType>(errorType), errorMessage));
> }
> 
>-extern "C" void JIT_OPERATION triggerReoptimizationNow(CodeBlock* codeBlock, OSRExitBase* exit)
>+extern "C" void JIT_OPERATION triggerReoptimizationNow(CodeBlock* codeBlock, CodeBlock* optimizedCodeBlock, OSRExitBase* exit)
> {
>     // It's sort of preferable that we don't GC while in here. Anyways, doing so wouldn't
>     // really be profitable.
>@@ -2830,6 +2830,9 @@ extern "C" void JIT_OPERATION triggerReo
> 
>     // If I am my own replacement, then reoptimization has already been triggered.
>     // This can happen in recursive functions.
>+    //
>+    // Note that even if optimizedCodeBlock is an FTLForOSREntry style CodeBlock, this condition is a
>+    // sure bet that we don't have anything else left to do.
>     if (codeBlock->replacement() == codeBlock) {
>         if (Options::verboseOSR())
>             dataLog(*codeBlock, ": Not reoptimizing because we've already been jettisoned.\n");
>@@ -2839,7 +2842,6 @@ extern "C" void JIT_OPERATION triggerReo
>     // Otherwise, the replacement must be optimized code. Use this as an opportunity
>     // to check our logic.
>     ASSERT(codeBlock->hasOptimizedReplacement());
>-    CodeBlock* optimizedCodeBlock = codeBlock->replacement();
>     ASSERT(JITCode::isOptimizingJIT(optimizedCodeBlock->jitType()));
>     
>     bool didTryToEnterIntoInlinedLoops = false;
>@@ -2864,7 +2866,7 @@ extern "C" void JIT_OPERATION triggerReo
>         codeBlock->optimizeAfterLongWarmUp();
>         return;
>     }
>-
>+    
>     optimizedCodeBlock->jettison(Profiler::JettisonDueToOSRExit, CountReoptimization);
> }
> 
>Index: Source/JavaScriptCore/dfg/DFGOperations.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOperations.h	(revision 231522)
>+++ Source/JavaScriptCore/dfg/DFGOperations.h	(working copy)
>@@ -266,7 +266,7 @@ JSCell* JIT_OPERATION operationNewObject
> 
> void JIT_OPERATION operationProcessTypeProfilerLogDFG(ExecState*) WTF_INTERNAL;
> 
>-void JIT_OPERATION triggerReoptimizationNow(CodeBlock*, OSRExitBase*) WTF_INTERNAL;
>+void JIT_OPERATION triggerReoptimizationNow(CodeBlock* baselineCodeBlock, CodeBlock* optiimzedCodeBlock, OSRExitBase*) WTF_INTERNAL;
> 
> #if USE(JSVALUE32_64)
> double JIT_OPERATION operationRandom(JSGlobalObject*);

Actions: View | Formatted Diff | Diff

Attachments on bug 185457: 339900 | 339929 | 340001