[JSC] Array#splice should skip result array creation if it is not use…

…d at all https://bugs.webkit.org/show_bug.cgi?id=259809 rdar://113367762 Reviewed by Keith Miller. This patch adds ArraySpliceExtract DFG node, which only accepts `array.splice(x, y)` form, which does not insert any elements. We leverage call_ignore_result etc.'s feedback information in DFG / FTL: we can see `array.splice(x, y)` result is not used, and DFG / FTL tells this hint to the operationArraySpliceExtract function. And then it can skip the result array creation when it is not used! This form is particularly frequently seen since `array.splice(x, y)` is a form of removing some elements in the middle of arrays. * Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h: (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects): * Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::handleIntrinsicCall): * Source/JavaScriptCore/dfg/DFGClobberize.h: (JSC::DFG::clobberize): * Source/JavaScriptCore/dfg/DFGDoesGC.cpp: (JSC::DFG::doesGC): * Source/JavaScriptCore/dfg/DFGFixupPhase.cpp: (JSC::DFG::FixupPhase::fixupNode): * Source/JavaScriptCore/dfg/DFGNode.h: (JSC::DFG::Node::hasHeapPrediction): * Source/JavaScriptCore/dfg/DFGNodeType.h: * Source/JavaScriptCore/dfg/DFGOperations.cpp: (JSC::DFG::JSC_DEFINE_JIT_OPERATION): * Source/JavaScriptCore/dfg/DFGOperations.h: * Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp: * Source/JavaScriptCore/dfg/DFGSafeToExecute.h: (JSC::DFG::safeToExecute): * Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp: * Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h: * Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp: (JSC::DFG::SpeculativeJIT::compile): * Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp: (JSC::DFG::SpeculativeJIT::compile): * Source/JavaScriptCore/ftl/FTLCapabilities.cpp: (JSC::FTL::canCompile): * Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compileNode): (JSC::FTL::DFG::LowerDFGToB3::compileArraySpliceExtract): * Source/JavaScriptCore/runtime/ArrayPrototype.cpp: (JSC::ArrayPrototype::finishCreation): (JSC::getProperty): Deleted. (JSC::setLength): Deleted. (JSC::shift): Deleted. (JSC::unshift): Deleted. * Source/JavaScriptCore/runtime/ArrayPrototypeInlines.h: (JSC::getProperty): (JSC::setLength): (JSC::shift): (JSC::unshift): * Source/JavaScriptCore/runtime/Intrinsic.h: Canonical link: https://commits.webkit.org/266591@main
WebKit · Aug 4, 2023 · ebb7275 · ebb7275
1 parent e6ac341
commit ebb7275
Show file tree

Hide file tree

Showing 21 changed files with 391 additions and 143 deletions.
diff --git a/JSTests/stress/array-split-ignore-result.js b/JSTests/stress/array-split-ignore-result.js
@@ -0,0 +1,30 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function test(array) {
+    array.splice(0, 3);
+}
+noInline(test);
+
+var count = 0;
+
+for (var i = 0; i < 1e4; ++i) {
+    var array = [0, 1, 2, 3];
+    var set = null;
+    Reflect.defineProperty(array, 0, {
+        get() {
+            ++count;
+            return 42;
+        },
+        set(value) {
+            set = value;
+        },
+        configurable: true,
+    });
+    test(array);
+    shouldBe(array.length, 1);
+    shouldBe(set, 3);
+}
+shouldBe(count, 1e4);
diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
@@ -2744,6 +2744,11 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
         break;
     }
 
+    case ArraySpliceExtract:
+        clobberWorld();
+        makeBytecodeTopForNode(node);
+        break;
+
     case ArrayIndexOf: {
         setNonCellTypeForNode(node, SpecInt32Only);
         break;

diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
@@ -2675,6 +2675,30 @@ auto ByteCodeParser::handleIntrinsicCall(Node* callee, Operand resultOperand, Ca
             return CallOptimizationResult::DidNothing;
         }
 
+        case ArraySpliceIntrinsic: {
+            // Currently we only handle extracting pattern `array.splice(x, y)` in a super fast manner.
+            if (argumentCountIncludingThis != 3)
+                return CallOptimizationResult::DidNothing;
+
+            if (m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadConstantCache)
+                || m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadCache)
+                || m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadType))
+                return CallOptimizationResult::DidNothing;
+
+            ArrayMode arrayMode = getArrayMode(Array::Read);
+            if (!arrayMode.isJSArray())
+                return CallOptimizationResult::DidNothing;
+
+            insertChecks();
+
+            Node* result = addToGraph(ArraySpliceExtract, OpInfo(), OpInfo(prediction),
+                get(virtualRegisterForArgumentIncludingThis(0, registerOffset)),
+                get(virtualRegisterForArgumentIncludingThis(1, registerOffset)),
+                get(virtualRegisterForArgumentIncludingThis(2, registerOffset)));
+            setResult(result);
+            return CallOptimizationResult::Inlined;
+        }
+
         case ArrayIndexOfIntrinsic: {
             if (argumentCountIncludingThis < 2)
                 return CallOptimizationResult::DidNothing;

diff --git a/Source/JavaScriptCore/dfg/DFGClobberize.h b/Source/JavaScriptCore/dfg/DFGClobberize.h
@@ -753,6 +753,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
     case DeleteByVal:
     case ArrayPush:
     case ArrayPop:
+    case ArraySpliceExtract:
     case Call:
     case DirectCall:
     case TailCallInlinedCaller:

diff --git a/Source/JavaScriptCore/dfg/DFGDoesGC.cpp b/Source/JavaScriptCore/dfg/DFGDoesGC.cpp
@@ -268,6 +268,7 @@ bool doesGC(Graph& graph, Node* node)
 #if ASSERT_ENABLED
     case ArrayPush:
     case ArrayPop:
+    case ArraySpliceExtract:
     case PushWithScope:
     case CreateActivation:
     case CreateDirectArguments:

diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -1533,6 +1533,13 @@ class FixupPhase : public Phase {
             break;
         }
 
+        case ArraySpliceExtract: {
+            fixEdge<ArrayUse>(node->child1());
+            fixEdge<Int32Use>(node->child2());
+            fixEdge<Int32Use>(node->child3());
+            break;
+        }
+
         case ArrayIndexOf:
             fixupArrayIndexOf(node);
             break;

diff --git a/Source/JavaScriptCore/dfg/DFGNode.h b/Source/JavaScriptCore/dfg/DFGNode.h
@@ -1943,6 +1943,7 @@ struct Node {
         case GetArgument:
         case ArrayPop:
         case ArrayPush:
+        case ArraySpliceExtract:
         case RegExpExec:
         case RegExpExecNonGlobalOrSticky:
         case RegExpTest:

diff --git a/Source/JavaScriptCore/dfg/DFGNodeType.h b/Source/JavaScriptCore/dfg/DFGNodeType.h
@@ -331,6 +331,7 @@ namespace JSC { namespace DFG {
     macro(ArrayPop, NodeResultJS | NodeMustGenerate) \
     macro(ArraySlice, NodeResultJS | NodeMustGenerate | NodeHasVarArgs) \
     macro(ArrayIndexOf, NodeResultInt32 | NodeHasVarArgs) \
+    macro(ArraySpliceExtract, NodeResultJS | NodeMustGenerate) \
     \
     /* Optimizations for regular expression matching. */\
     macro(RegExpExec, NodeResultJS | NodeMustGenerate) \

diff --git a/Source/JavaScriptCore/dfg/DFGOperations.cpp b/Source/JavaScriptCore/dfg/DFGOperations.cpp
@@ -1240,7 +1240,126 @@ JSC_DEFINE_JIT_OPERATION(operationArrayPopAndRecoverLength, EncodedJSValue, (JSG
 
     return JSValue::encode(array->pop(globalObject));
 }
-
+
+JSC_DEFINE_JIT_OPERATION(operationArraySpliceExtract, EncodedJSValue, (JSGlobalObject* globalObject, JSArray* base, int32_t start, int32_t deleteCount, unsigned refCount))
+{
+    VM& vm = globalObject->vm();
+    CallFrame* callFrame = DECLARE_CALL_FRAME(vm);
+    JITOperationPrologueCallFrameTracer tracer(vm, callFrame);
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    uint64_t length = base->length();
+    if (!length) {
+        scope.release();
+        setLength(globalObject, vm, base, length);
+        constexpr bool throwException = true;
+        base->setLength(globalObject, 0, throwException);
+        return JSValue::encode(jsUndefined());
+    }
+
+    uint64_t actualStart = 0;
+    int64_t startInt64 = start;
+    if (startInt64 < 0) {
+        startInt64 += length;
+        actualStart = startInt64 < 0 ? 0 : static_cast<uint64_t>(startInt64);
+    } else
+        actualStart = std::min(static_cast<uint64_t>(startInt64), length);
+
+    uint64_t actualDeleteCount = 0;
+    if (deleteCount < 0)
+        actualDeleteCount = 0;
+    else if (deleteCount > static_cast<int64_t>(length - actualStart))
+        actualDeleteCount = length - actualStart;
+    else
+        actualDeleteCount = static_cast<uint64_t>(deleteCount);
+
+    std::pair<SpeciesConstructResult, JSObject*> speciesResult = speciesConstructArray(globalObject, base, actualDeleteCount);
+    EXCEPTION_ASSERT(!!scope.exception() == (speciesResult.first == SpeciesConstructResult::Exception));
+    if (speciesResult.first == SpeciesConstructResult::Exception)
+        return JSValue::encode(jsUndefined());
+
+    JSValue result;
+    if (LIKELY(speciesResult.first == SpeciesConstructResult::FastPath)) {
+        // DFG / FTL tells the hint that the result array is not used at all.
+        // If this condition is met, we can skip creation of this array completely.
+        auto canFastSliceWithoutSideEffect = [](JSGlobalObject* globalObject, JSArray* base, uint64_t count) {
+            auto arrayType = base->indexingType() | IsArray;
+            switch (arrayType) {
+            case ArrayWithDouble:
+            case ArrayWithInt32:
+            case ArrayWithContiguous: {
+                if (count >= MIN_SPARSE_ARRAY_INDEX || base->structure()->holesMustForwardToPrototype(base))
+                    return false;
+
+                Structure* resultStructure = globalObject->arrayStructureForIndexingTypeDuringAllocation(arrayType);
+                if (UNLIKELY(hasAnyArrayStorage(resultStructure->indexingType())))
+                    return false;
+
+                return true;
+            }
+            case ArrayWithArrayStorage: {
+                if (count >= MIN_SPARSE_ARRAY_INDEX || base->structure()->holesMustForwardToPrototype(base))
+                    return false;
+
+                Structure* resultStructure = globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous);
+                if (UNLIKELY(hasAnyArrayStorage(resultStructure->indexingType())))
+                    return false;
+                return true;
+            }
+            case ArrayWithUndecided: {
+                return true;
+            }
+            default:
+                return false;
+            }
+        };
+
+        if (!refCount && canFastSliceWithoutSideEffect(globalObject, base, actualDeleteCount))
+            result = jsUndefined();
+        else {
+            result = JSArray::fastSlice(globalObject, base, actualStart, actualDeleteCount);
+            RETURN_IF_EXCEPTION(scope, { });
+        }
+    }
+
+    if (UNLIKELY(!result)) {
+        JSObject* resultObject = nullptr;
+        if (speciesResult.first == SpeciesConstructResult::CreatedObject)
+            resultObject = speciesResult.second;
+        else {
+            if (UNLIKELY(actualDeleteCount > std::numeric_limits<uint32_t>::max())) {
+                throwRangeError(globalObject, scope, LengthExceededTheMaximumArrayLengthError);
+                return { };
+            }
+            resultObject = JSArray::tryCreate(vm, globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithUndecided), static_cast<uint32_t>(actualDeleteCount));
+            if (UNLIKELY(!resultObject)) {
+                throwOutOfMemoryError(globalObject, scope);
+                return { };
+            }
+        }
+        for (uint64_t k = 0; k < actualDeleteCount; ++k) {
+            JSValue v = getProperty(globalObject, base, k + actualStart);
+            RETURN_IF_EXCEPTION(scope, { });
+            if (UNLIKELY(!v))
+                continue;
+            resultObject->putDirectIndex(globalObject, k, v, 0, PutDirectIndexShouldThrow);
+            RETURN_IF_EXCEPTION(scope, { });
+        }
+        setLength(globalObject, vm, resultObject, actualDeleteCount);
+        RETURN_IF_EXCEPTION(scope, { });
+        result = resultObject;
+    }
+
+    if (actualDeleteCount) {
+        shift<JSArray::ShiftCountForSplice>(globalObject, base, actualStart, actualDeleteCount, 0, length);
+        RETURN_IF_EXCEPTION(scope, { });
+    }
+
+    scope.release();
+    setLength(globalObject, vm, base, length - actualDeleteCount);
+    return JSValue::encode(result);
+}
+
 JSC_DEFINE_JIT_OPERATION(operationRegExpExecString, EncodedJSValue, (JSGlobalObject* globalObject, RegExpObject* regExpObject, JSString* argument))
 {
     SuperSamplerScope superSamplerScope(false);

diff --git a/Source/JavaScriptCore/dfg/DFGOperations.h b/Source/JavaScriptCore/dfg/DFGOperations.h
@@ -201,6 +201,7 @@ JSC_DECLARE_JIT_OPERATION(operationArrayPushDouble, EncodedJSValue, (JSGlobalObj
 JSC_DECLARE_JIT_OPERATION(operationArrayPushDoubleMultiple, EncodedJSValue, (JSGlobalObject*, JSArray*, void* buffer, int32_t elementCount));
 JSC_DECLARE_JIT_OPERATION(operationArrayPop, EncodedJSValue, (JSGlobalObject*, JSArray*));
 JSC_DECLARE_JIT_OPERATION(operationArrayPopAndRecoverLength, EncodedJSValue, (JSGlobalObject*, JSArray*));
+JSC_DECLARE_JIT_OPERATION(operationArraySpliceExtract, EncodedJSValue, (JSGlobalObject*, JSArray*, int32_t start, int32_t deleteCount, unsigned refCount));
 JSC_DECLARE_JIT_OPERATION(operationRegExpExecString, EncodedJSValue, (JSGlobalObject*, RegExpObject*, JSString*));
 JSC_DECLARE_JIT_OPERATION(operationRegExpExec, EncodedJSValue, (JSGlobalObject*, RegExpObject*, EncodedJSValue));
 JSC_DECLARE_JIT_OPERATION(operationRegExpExecGeneric, EncodedJSValue, (JSGlobalObject*, EncodedJSValue, EncodedJSValue));

diff --git a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
@@ -978,6 +978,7 @@ class PredictionPropagationPhase : public Phase {
         case GetByValMegamorphic:
         case ArrayPop:
         case ArrayPush:
+        case ArraySpliceExtract:
         case RegExpExec:
         case RegExpExecNonGlobalOrSticky:
         case RegExpTest:

diff --git a/Source/JavaScriptCore/dfg/DFGSafeToExecute.h b/Source/JavaScriptCore/dfg/DFGSafeToExecute.h
@@ -725,6 +725,7 @@ bool safeToExecute(AbstractStateType& state, Graph& graph, Node* node, bool igno
     case StringLocaleCompare:
     case FunctionBind:
     case DateSetTime:
+    case ArraySpliceExtract:
         return false;
 
     case StringReplaceString:

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -10455,6 +10455,31 @@ void SpeculativeJIT::compileArraySlice(Node* node)
     cellResult(resultGPR, node);
 }
 
+void SpeculativeJIT::compileArraySpliceExtract(Node* node)
+{
+    unsigned refCount = node->refCount();
+    bool mustGenerate = node->mustGenerate();
+    if (mustGenerate)
+        --refCount;
+
+    SpeculateCellOperand base(this, node->child1());
+    SpeculateInt32Operand start(this, node->child2());
+    SpeculateInt32Operand deleteCount(this, node->child3());
+
+    GPRReg baseGPR = base.gpr();
+    GPRReg startGPR = start.gpr();
+    GPRReg deleteCountGPR = deleteCount.gpr();
+
+    speculateArray(node->child1(), baseGPR);
+
+    flushRegisters();
+    JSValueRegsFlushedCallResult result(this);
+    JSValueRegs resultRegs = result.regs();
+    callOperation(operationArraySpliceExtract, resultRegs, LinkableConstant::globalObject(*this, node), baseGPR, startGPR, deleteCountGPR, TrustedImm32(refCount));
+    exceptionCheck();
+    jsValueResult(resultRegs, node);
+}
+
 void SpeculativeJIT::compileArrayIndexOf(Node* node)
 {
     ASSERT(node->op() == ArrayIndexOf);

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
@@ -1525,6 +1525,7 @@ class SpeculativeJIT : public JITCompiler {
     void compileNewArrayWithSpread(Node*);
     void compileGetRestLength(Node*);
     void compileArraySlice(Node*);
+    void compileArraySpliceExtract(Node*);
     void compileArrayIndexOf(Node*);
     void compileArrayPush(Node*);
     void compileNotifyWrite(Node*);

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
@@ -2900,6 +2900,11 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
 
+    case ArraySpliceExtract: {
+        compileArraySpliceExtract(node);
+        break;
+    }
+
     case ArrayIndexOf: {
         compileArrayIndexOf(node);
         break;

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
@@ -3930,6 +3930,11 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
 
+    case ArraySpliceExtract: {
+        compileArraySpliceExtract(node);
+        break;
+    }
+
     case ArrayIndexOf: {
         compileArrayIndexOf(node);
         break;

diff --git a/Source/JavaScriptCore/ftl/FTLCapabilities.cpp b/Source/JavaScriptCore/ftl/FTLCapabilities.cpp
@@ -399,6 +399,7 @@ inline CapabilityLevel canCompile(Node* node)
     case CallDOM:
     case CallDOMGetter:
     case ArraySlice:
+    case ArraySpliceExtract:
     case ArrayIndexOf:
     case ArrayPop:
     case ArrayPush:

diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
@@ -1134,6 +1134,9 @@ class LowerDFGToB3 {
         case ArraySlice:
             compileArraySlice();
             break;
+        case ArraySpliceExtract:
+            compileArraySpliceExtract();
+            break;
         case ArrayIndexOf:
             compileArrayIndexOf();
             break;
@@ -7330,6 +7333,20 @@ IGNORE_CLANG_WARNINGS_END
         setJSValue(arrayResult.array);
     }
 
+    void compileArraySpliceExtract()
+    {
+        JSGlobalObject* globalObject = m_graph.globalObjectFor(m_origin.semantic);
+
+        unsigned refCount = m_node->refCount();
+        bool mustGenerate = m_node->mustGenerate();
+        if (mustGenerate)
+            --refCount;
+
+        LValue base = lowCell(m_node->child1());
+        speculateArray(m_node->child1(), base);
+        setJSValue(vmCall(Int64, operationArraySpliceExtract, weakPointer(globalObject), base, lowInt32(m_node->child2()), lowInt32(m_node->child3()), m_out.constInt32(refCount)));
+    }
+
     void compileArrayIndexOf()
     {
         JSGlobalObject* globalObject = m_graph.globalObjectFor(m_origin.semantic);