[JSC] DFG terminal's liveness should respect caller's opcodeID
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Nov 2019 19:55:27 +0000 (19:55 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Nov 2019 19:55:27 +0000 (19:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=204317

Reviewed by Saam Barati.

JSTests:

* stress/call-var-args-phantom-arguments-handler-strict.js: Added.
(shouldBe):
(inlined):
(test):
* stress/call-var-args-phantom-arguments-handler.js: Added.
(shouldBe):
(inlined):
(test):
* stress/call-var-args-phantom-arguments-strict.js: Added.
(shouldBe):
(inlined):
(test):
* stress/call-var-args-phantom-arguments.js: Added.
(shouldBe):
(inlined):
(test):
* stress/derived-class-construct-varargs.js: Added.
(shouldThrow):
(B):
* stress/tail-call-var-args-phantom-arguments-handler-strict.js: Added.
(shouldBe):
(inlined):
(test):
* stress/tail-call-var-args-phantom-arguments-handler.js: Added.
(shouldBe):
(inlined):
(test):
* stress/tail-call-var-args-phantom-arguments-strict.js: Added.
(shouldBe):
(inlined):
(test):
* stress/tail-call-var-args-phantom-arguments.js: Added.
(shouldBe):
(inlined):
(test):

Source/JavaScriptCore:

Let's consider the following example, which is freqneutly seen in Speedometer2/EmberJS-Debug.

    "use strict";

    function assertImpl(cond)
    {
        if (!cond)
            throw new Error();
    }

    function assert()
    {
        assertImpl.apply(undefined, arguments);
    }
    noInline(assert);

When compiling `throw`, we emit a terminal node and put Phantom/PhantomLocal based on the bytecode liveness.
When collecting liveness for each frame, we use the liveness information of the bytecode `op_call_varargs` in assert function.
This means that op_call_varargs's uses are considered as live (like, `arguments` in this example).
But it is not necessary to mark it "live": if we are in assertImpl, `arguments` is already loaded into the stack, and we no longer
use `arguments` when exiting, and the execution after the exit. Marking this `arguments` live makes this `arguments` allocated
in DFG, but this is wasteful.

In this patch, we introduce BeforeUse and AfterUse concept into bytecode liveness information. And use AfterUse information when
collecting liveness in the caller's frame in DFG. We only enable this for varargs for now since (1) applying this to the other ones
is not profitable, and (2) we need to be careful to make stack arguments live to allow materialization of arguments objects.
In op_call_varargs / op_tail_call_varargs / op_construct_varargs cases, uses are happen only for |callee|, |this|, and |arguments|.
And these are no longer necessary after calling.

We don't use liveness information in the next bytecode since it misses uses marked by exception handlers.

* bytecode/BytecodeLivenessAnalysis.cpp:
(JSC::BytecodeLivenessAnalysis::computeFullLiveness):
* bytecode/BytecodeLivenessAnalysis.h:
(JSC::BytecodeLivenessAnalysis::graph):
* bytecode/BytecodeLivenessAnalysisInlines.h:
(JSC::BytecodeLivenessPropagation::stepOverInstructionDef):
(JSC::BytecodeLivenessPropagation::stepOverInstructionUse):
(JSC::BytecodeLivenessPropagation::stepOverInstructionUseInExceptionHandler):
(JSC::BytecodeLivenessPropagation::stepOverInstruction):
* bytecode/BytecodeUseDef.h:
(JSC::computeUsesForBytecodeIndex):
(JSC::computeDefsForBytecodeIndex):
* bytecode/FullBytecodeLiveness.h:
(JSC::FullBytecodeLiveness::getLiveness const):
(JSC::FullBytecodeLiveness::operandIsLive const):
* bytecompiler/BytecodeGenerator.cpp:
(JSC::ForInContext::finalize):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::flushForTerminalImpl):
* dfg/DFGForAllKills.h:
(JSC::DFG::forAllKilledOperands):
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::isLiveInBytecode):
* dfg/DFGGraph.h:
(JSC::DFG::Graph::forAllLocalsLiveInBytecode):
(JSC::DFG::Graph::appropriateLivenessCalculationPoint):
* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@252789 268f45cc-cd09-0410-ab3c-d52691b4dbfc

23 files changed:
JSTests/ChangeLog
JSTests/stress/call-var-args-phantom-arguments-handler-strict.js [new file with mode: 0644]
JSTests/stress/call-var-args-phantom-arguments-handler.js [new file with mode: 0644]
JSTests/stress/call-var-args-phantom-arguments-strict.js [new file with mode: 0644]
JSTests/stress/call-var-args-phantom-arguments.js [new file with mode: 0644]
JSTests/stress/derived-class-construct-varargs.js [new file with mode: 0644]
JSTests/stress/tail-call-var-args-phantom-arguments-handler-strict.js [new file with mode: 0644]
JSTests/stress/tail-call-var-args-phantom-arguments-handler.js [new file with mode: 0644]
JSTests/stress/tail-call-var-args-phantom-arguments-strict.js [new file with mode: 0644]
JSTests/stress/tail-call-var-args-phantom-arguments.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp
Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h
Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysisInlines.h
Source/JavaScriptCore/bytecode/BytecodeUseDef.h
Source/JavaScriptCore/bytecode/FullBytecodeLiveness.h
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGForAllKills.h
Source/JavaScriptCore/dfg/DFGGraph.cpp
Source/JavaScriptCore/dfg/DFGGraph.h
Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm
Source/JavaScriptCore/llint/LowLevelInterpreter64.asm

index 161cdd6..3a061d0 100644 (file)
@@ -1,3 +1,46 @@
+2019-11-22  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] DFG terminal's liveness should respect caller's opcodeID
+        https://bugs.webkit.org/show_bug.cgi?id=204317
+
+        Reviewed by Saam Barati.
+
+        * stress/call-var-args-phantom-arguments-handler-strict.js: Added.
+        (shouldBe):
+        (inlined):
+        (test):
+        * stress/call-var-args-phantom-arguments-handler.js: Added.
+        (shouldBe):
+        (inlined):
+        (test):
+        * stress/call-var-args-phantom-arguments-strict.js: Added.
+        (shouldBe):
+        (inlined):
+        (test):
+        * stress/call-var-args-phantom-arguments.js: Added.
+        (shouldBe):
+        (inlined):
+        (test):
+        * stress/derived-class-construct-varargs.js: Added.
+        (shouldThrow):
+        (B):
+        * stress/tail-call-var-args-phantom-arguments-handler-strict.js: Added.
+        (shouldBe):
+        (inlined):
+        (test):
+        * stress/tail-call-var-args-phantom-arguments-handler.js: Added.
+        (shouldBe):
+        (inlined):
+        (test):
+        * stress/tail-call-var-args-phantom-arguments-strict.js: Added.
+        (shouldBe):
+        (inlined):
+        (test):
+        * stress/tail-call-var-args-phantom-arguments.js: Added.
+        (shouldBe):
+        (inlined):
+        (test):
+
 2019-11-22  Guillaume Emont  <guijemont@igalia.com>
 
         Skip tests that fail on arm and mips devices
diff --git a/JSTests/stress/call-var-args-phantom-arguments-handler-strict.js b/JSTests/stress/call-var-args-phantom-arguments-handler-strict.js
new file mode 100644 (file)
index 0000000..ce37928
--- /dev/null
@@ -0,0 +1,34 @@
+"use strict";
+
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function inlined(cond)
+{
+    if (!cond)
+        throw new Error("OK");
+}
+function test()
+{
+    try {
+        inlined.apply(undefined, arguments);
+    } catch (error) {
+        shouldBe(String(error), `Error: OK`);
+        shouldBe(arguments.length, 3);
+        shouldBe(arguments[0], 0);
+        shouldBe(arguments[1], 2);
+        shouldBe(arguments[2], 3);
+    }
+}
+noInline(test);
+
+for (var i = 0; i < 1e7; ++i) {
+    test(1, 2, 3);
+}
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
diff --git a/JSTests/stress/call-var-args-phantom-arguments-handler.js b/JSTests/stress/call-var-args-phantom-arguments-handler.js
new file mode 100644 (file)
index 0000000..0f67443
--- /dev/null
@@ -0,0 +1,32 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function inlined(cond)
+{
+    if (!cond)
+        throw new Error("OK");
+}
+function test()
+{
+    try {
+        inlined.apply(undefined, arguments);
+    } catch (error) {
+        shouldBe(String(error), `Error: OK`);
+        shouldBe(arguments.length, 3);
+        shouldBe(arguments[0], 0);
+        shouldBe(arguments[1], 2);
+        shouldBe(arguments[2], 3);
+    }
+}
+noInline(test);
+
+for (var i = 0; i < 1e7; ++i) {
+    test(1, 2, 3);
+}
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
diff --git a/JSTests/stress/call-var-args-phantom-arguments-strict.js b/JSTests/stress/call-var-args-phantom-arguments-strict.js
new file mode 100644 (file)
index 0000000..a260556
--- /dev/null
@@ -0,0 +1,30 @@
+"use strict";
+
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function inlined(cond)
+{
+    if (!cond)
+        throw new Error("OK");
+}
+function test()
+{
+    try {
+        inlined.apply(undefined, arguments);
+    } catch (error) {
+        shouldBe(String(error), `Error: OK`);
+    }
+}
+noInline(test);
+
+for (var i = 0; i < 1e7; ++i) {
+    test(1, 2, 3);
+}
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
diff --git a/JSTests/stress/call-var-args-phantom-arguments.js b/JSTests/stress/call-var-args-phantom-arguments.js
new file mode 100644 (file)
index 0000000..750f1e1
--- /dev/null
@@ -0,0 +1,28 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function inlined(cond)
+{
+    if (!cond)
+        throw new Error("OK");
+}
+function test()
+{
+    try {
+        inlined.apply(undefined, arguments);
+    } catch (error) {
+        shouldBe(String(error), `Error: OK`);
+    }
+}
+noInline(test);
+
+for (var i = 0; i < 1e7; ++i) {
+    test(1, 2, 3);
+}
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
diff --git a/JSTests/stress/derived-class-construct-varargs.js b/JSTests/stress/derived-class-construct-varargs.js
new file mode 100644 (file)
index 0000000..256fb96
--- /dev/null
@@ -0,0 +1,28 @@
+function shouldThrow(func, errorMessage) {
+    var errorThrown = false;
+    var error = null;
+    try {
+        func();
+    } catch (e) {
+        errorThrown = true;
+        error = e;
+    }
+    if (!errorThrown)
+        throw new Error('not thrown');
+    if (String(error) !== errorMessage)
+        throw new Error(`bad error: ${String(error)}`);
+}
+
+class A {
+    constructor(cond)
+    {
+        if (!cond)
+            throw new Error("OK");
+    }
+}
+
+class B extends A {}
+noInline(B);
+for (var i = 0; i < 1e6; ++i)
+    new B(1);
+shouldThrow(() => new B(0), `Error: OK`);
diff --git a/JSTests/stress/tail-call-var-args-phantom-arguments-handler-strict.js b/JSTests/stress/tail-call-var-args-phantom-arguments-handler-strict.js
new file mode 100644 (file)
index 0000000..9a7417e
--- /dev/null
@@ -0,0 +1,34 @@
+"use strict";
+
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function inlined(cond)
+{
+    if (!cond)
+        throw new Error("OK");
+}
+function test()
+{
+    try {
+        return inlined.apply(undefined, arguments);
+    } catch (error) {
+        shouldBe(String(error), `Error: OK`);
+        shouldBe(arguments.length, 3);
+        shouldBe(arguments[0], 0);
+        shouldBe(arguments[1], 2);
+        shouldBe(arguments[2], 3);
+    }
+}
+noInline(test);
+
+for (var i = 0; i < 1e7; ++i) {
+    test(1, 2, 3);
+}
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
diff --git a/JSTests/stress/tail-call-var-args-phantom-arguments-handler.js b/JSTests/stress/tail-call-var-args-phantom-arguments-handler.js
new file mode 100644 (file)
index 0000000..5b0d379
--- /dev/null
@@ -0,0 +1,32 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function inlined(cond)
+{
+    if (!cond)
+        throw new Error("OK");
+}
+function test()
+{
+    try {
+        return inlined.apply(undefined, arguments);
+    } catch (error) {
+        shouldBe(String(error), `Error: OK`);
+        shouldBe(arguments.length, 3);
+        shouldBe(arguments[0], 0);
+        shouldBe(arguments[1], 2);
+        shouldBe(arguments[2], 3);
+    }
+}
+noInline(test);
+
+for (var i = 0; i < 1e7; ++i) {
+    test(1, 2, 3);
+}
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
diff --git a/JSTests/stress/tail-call-var-args-phantom-arguments-strict.js b/JSTests/stress/tail-call-var-args-phantom-arguments-strict.js
new file mode 100644 (file)
index 0000000..447a5db
--- /dev/null
@@ -0,0 +1,30 @@
+"use strict";
+
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function inlined(cond)
+{
+    if (!cond)
+        throw new Error("OK");
+}
+function test()
+{
+    try {
+        return inlined.apply(undefined, arguments);
+    } catch (error) {
+        shouldBe(String(error), `Error: OK`);
+    }
+}
+noInline(test);
+
+for (var i = 0; i < 1e7; ++i) {
+    test(1, 2, 3);
+}
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
diff --git a/JSTests/stress/tail-call-var-args-phantom-arguments.js b/JSTests/stress/tail-call-var-args-phantom-arguments.js
new file mode 100644 (file)
index 0000000..d95fd63
--- /dev/null
@@ -0,0 +1,28 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function inlined(cond)
+{
+    if (!cond)
+        throw new Error("OK");
+}
+function test()
+{
+    try {
+        return inlined.apply(undefined, arguments);
+    } catch (error) {
+        shouldBe(String(error), `Error: OK`);
+    }
+}
+noInline(test);
+
+for (var i = 0; i < 1e7; ++i) {
+    test(1, 2, 3);
+}
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
+test(0, 2, 3);
index 2be9630..4713a00 100644 (file)
@@ -1,3 +1,70 @@
+2019-11-22  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] DFG terminal's liveness should respect caller's opcodeID
+        https://bugs.webkit.org/show_bug.cgi?id=204317
+
+        Reviewed by Saam Barati.
+
+        Let's consider the following example, which is freqneutly seen in Speedometer2/EmberJS-Debug.
+
+            "use strict";
+
+            function assertImpl(cond)
+            {
+                if (!cond)
+                    throw new Error();
+            }
+
+            function assert()
+            {
+                assertImpl.apply(undefined, arguments);
+            }
+            noInline(assert);
+
+        When compiling `throw`, we emit a terminal node and put Phantom/PhantomLocal based on the bytecode liveness.
+        When collecting liveness for each frame, we use the liveness information of the bytecode `op_call_varargs` in assert function.
+        This means that op_call_varargs's uses are considered as live (like, `arguments` in this example).
+        But it is not necessary to mark it "live": if we are in assertImpl, `arguments` is already loaded into the stack, and we no longer
+        use `arguments` when exiting, and the execution after the exit. Marking this `arguments` live makes this `arguments` allocated
+        in DFG, but this is wasteful.
+
+        In this patch, we introduce BeforeUse and AfterUse concept into bytecode liveness information. And use AfterUse information when
+        collecting liveness in the caller's frame in DFG. We only enable this for varargs for now since (1) applying this to the other ones
+        is not profitable, and (2) we need to be careful to make stack arguments live to allow materialization of arguments objects.
+        In op_call_varargs / op_tail_call_varargs / op_construct_varargs cases, uses are happen only for |callee|, |this|, and |arguments|.
+        And these are no longer necessary after calling.
+
+        We don't use liveness information in the next bytecode since it misses uses marked by exception handlers.
+
+        * bytecode/BytecodeLivenessAnalysis.cpp:
+        (JSC::BytecodeLivenessAnalysis::computeFullLiveness):
+        * bytecode/BytecodeLivenessAnalysis.h:
+        (JSC::BytecodeLivenessAnalysis::graph):
+        * bytecode/BytecodeLivenessAnalysisInlines.h:
+        (JSC::BytecodeLivenessPropagation::stepOverInstructionDef):
+        (JSC::BytecodeLivenessPropagation::stepOverInstructionUse):
+        (JSC::BytecodeLivenessPropagation::stepOverInstructionUseInExceptionHandler):
+        (JSC::BytecodeLivenessPropagation::stepOverInstruction):
+        * bytecode/BytecodeUseDef.h:
+        (JSC::computeUsesForBytecodeIndex):
+        (JSC::computeDefsForBytecodeIndex):
+        * bytecode/FullBytecodeLiveness.h:
+        (JSC::FullBytecodeLiveness::getLiveness const):
+        (JSC::FullBytecodeLiveness::operandIsLive const):
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::ForInContext::finalize):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::flushForTerminalImpl):
+        * dfg/DFGForAllKills.h:
+        (JSC::DFG::forAllKilledOperands):
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::isLiveInBytecode):
+        * dfg/DFGGraph.h:
+        (JSC::DFG::Graph::forAllLocalsLiveInBytecode):
+        (JSC::DFG::Graph::appropriateLivenessCalculationPoint):
+        * llint/LowLevelInterpreter32_64.asm:
+        * llint/LowLevelInterpreter64.asm:
+
 2019-11-22  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Unreviewed. Fix GTK/WPE debug build after r252770
index a1c2821..918e1d2 100644 (file)
@@ -67,7 +67,8 @@ void BytecodeLivenessAnalysis::computeFullLiveness(CodeBlock* codeBlock, FullByt
 {
     FastBitVector out;
 
-    result.m_map.resize(codeBlock->instructions().size());
+    result.m_beforeUseVector.resize(codeBlock->instructions().size());
+    result.m_afterUseVector.resize(codeBlock->instructions().size());
     
     for (std::unique_ptr<BytecodeBasicBlock>& block : m_graph.basicBlocksInReverseOrder()) {
         if (block->isEntryBlock() || block->isExitBlock())
@@ -75,10 +76,25 @@ void BytecodeLivenessAnalysis::computeFullLiveness(CodeBlock* codeBlock, FullByt
         
         out = block->out();
         
+        auto use = [&] (unsigned bitIndex) {
+            // This is the use functor, so we set the bit.
+            out[bitIndex] = true;
+        };
+
+        auto def = [&] (unsigned bitIndex) {
+            // This is the def functor, so we clear the bit.
+            out[bitIndex] = false;
+        };
+
+        auto& instructions = codeBlock->instructions();
         for (unsigned i = block->offsets().size(); i--;) {
-            unsigned bytecodeOffset = block->offsets()[i];
-            stepOverInstruction(codeBlock, codeBlock->instructions(), m_graph, BytecodeIndex(bytecodeOffset), out);
-            result.m_map[bytecodeOffset] = out;
+            BytecodeIndex bytecodeIndex = BytecodeIndex(block->offsets()[i]);
+
+            stepOverInstructionDef(codeBlock, instructions, m_graph, bytecodeIndex, def);
+            stepOverInstructionUseInExceptionHandler(codeBlock, instructions, m_graph, bytecodeIndex, use);
+            result.m_afterUseVector[bytecodeIndex.offset()] = out; // AfterUse point.
+            stepOverInstructionUse(codeBlock, instructions, m_graph, bytecodeIndex, use);
+            result.m_beforeUseVector[bytecodeIndex.offset()] = out; // BeforeUse point.
         }
     }
 }
index eac185d..7379b3c 100644 (file)
@@ -35,19 +35,44 @@ namespace JSC {
 class BytecodeKills;
 class FullBytecodeLiveness;
 
+// We model our bytecode effects like the following and insert the liveness calculation points.
+//
+// <- BeforeUse
+//     Use
+// <- AfterUse
+//     Use by exception handlers
+//     Def
+enum class LivenessCalculationPoint : uint8_t {
+    BeforeUse,
+    AfterUse,
+};
+
 class BytecodeLivenessPropagation {
-protected:
-    template<typename CodeBlockType, typename UseFunctor, typename DefFunctor> void stepOverInstruction(CodeBlockType*, const InstructionStream&, BytecodeGraph&, BytecodeIndex, const UseFunctor&, const DefFunctor&);
+public:
+    template<typename CodeBlockType, typename UseFunctor>
+    static void stepOverInstructionUse(CodeBlockType*, const InstructionStream&, BytecodeGraph&, BytecodeIndex, const UseFunctor&);
+    template<typename CodeBlockType, typename UseFunctor>
+    static void stepOverInstructionUseInExceptionHandler(CodeBlockType*, const InstructionStream&, BytecodeGraph&, BytecodeIndex, const UseFunctor&);
+    template<typename CodeBlockType, typename DefFunctor>
+    static void stepOverInstructionDef(CodeBlockType*, const InstructionStream&, BytecodeGraph&, BytecodeIndex, const DefFunctor&);
 
-    template<typename CodeBlockType> void stepOverInstruction(CodeBlockType*, const InstructionStream&, BytecodeGraph&, BytecodeIndex, FastBitVector& out);
+    template<typename CodeBlockType, typename UseFunctor, typename DefFunctor>
+    static void stepOverInstruction(CodeBlockType*, const InstructionStream&, BytecodeGraph&, BytecodeIndex, const UseFunctor&, const DefFunctor&);
 
-    template<typename CodeBlockType, typename Instructions> bool computeLocalLivenessForBytecodeIndex(CodeBlockType*, const Instructions&, BytecodeGraph&, BytecodeBasicBlock*, BytecodeIndex, FastBitVector& result);
+    template<typename CodeBlockType>
+    static void stepOverInstruction(CodeBlockType*, const InstructionStream&, BytecodeGraph&, BytecodeIndex, FastBitVector& out);
 
-    template<typename CodeBlockType, typename Instructions> bool computeLocalLivenessForBlock(CodeBlockType*, const Instructions&, BytecodeGraph&, BytecodeBasicBlock*);
+    template<typename CodeBlockType, typename Instructions>
+    static bool computeLocalLivenessForBytecodeIndex(CodeBlockType*, const Instructions&, BytecodeGraph&, BytecodeBasicBlock*, BytecodeIndex, FastBitVector& result);
 
-    template<typename CodeBlockType, typename Instructions> FastBitVector getLivenessInfoAtBytecodeIndex(CodeBlockType*, const Instructions&, BytecodeGraph&, BytecodeIndex);
+    template<typename CodeBlockType, typename Instructions>
+    static bool computeLocalLivenessForBlock(CodeBlockType*, const Instructions&, BytecodeGraph&, BytecodeBasicBlock*);
 
-    template<typename CodeBlockType, typename Instructions> void runLivenessFixpoint(CodeBlockType*, const Instructions&, BytecodeGraph&);
+    template<typename CodeBlockType, typename Instructions>
+    static FastBitVector getLivenessInfoAtBytecodeIndex(CodeBlockType*, const Instructions&, BytecodeGraph&, BytecodeIndex);
+
+    template<typename CodeBlockType, typename Instructions>
+    static void runLivenessFixpoint(CodeBlockType*, const Instructions&, BytecodeGraph&);
 };
 
 class BytecodeLivenessAnalysis : private BytecodeLivenessPropagation {
@@ -62,6 +87,8 @@ public:
     void computeFullLiveness(CodeBlock*, FullBytecodeLiveness& result);
     void computeKills(CodeBlock*, BytecodeKills& result);
 
+    BytecodeGraph& graph() { return m_graph; }
+
 private:
     void dumpResults(CodeBlock*);
 
index 7a90032..3d12e19 100644 (file)
@@ -58,43 +58,33 @@ inline bool isValidRegisterForLiveness(VirtualRegister operand)
     return operand.isLocal();
 }
 
-// Simplified interface to bytecode use/def, which determines defs first and then uses, and includes
-// exception handlers in the uses.
-template<typename CodeBlockType, typename UseFunctor, typename DefFunctor>
-inline void BytecodeLivenessPropagation::stepOverInstruction(CodeBlockType* codeBlock, const InstructionStream& instructions, BytecodeGraph& graph, BytecodeIndex bytecodeIndex, const UseFunctor& use, const DefFunctor& def)
+template<typename CodeBlockType, typename DefFunctor>
+inline void BytecodeLivenessPropagation::stepOverInstructionDef(CodeBlockType* codeBlock, const InstructionStream& instructions, BytecodeGraph&, BytecodeIndex bytecodeIndex, const DefFunctor& def)
 {
-    // This abstractly execute the instruction in reverse. Instructions logically first use operands and
-    // then define operands. This logical ordering is necessary for operations that use and def the same
-    // operand, like:
-    //
-    //     op_add loc1, loc1, loc2
-    //
-    // The use of loc1 happens before the def of loc1. That's a semantic requirement since the add
-    // operation cannot travel forward in time to read the value that it will produce after reading that
-    // value. Since we are executing in reverse, this means that we must do defs before uses (reverse of
-    // uses before defs).
-    //
-    // Since this is a liveness analysis, this ordering ends up being particularly important: if we did
-    // uses before defs, then the add operation above would appear to not have loc1 live, since we'd
-    // first add it to the out set (the use), and then we'd remove it (the def).
-
     auto* instruction = instructions.at(bytecodeIndex).ptr();
-    OpcodeID opcodeID = instruction->opcodeID();
-
     computeDefsForBytecodeIndex(
-        codeBlock, opcodeID, instruction,
+        codeBlock, instruction,
         [&] (VirtualRegister operand) {
             if (isValidRegisterForLiveness(operand))
                 def(operand.toLocal());
         });
+}
 
+template<typename CodeBlockType, typename UseFunctor>
+inline void BytecodeLivenessPropagation::stepOverInstructionUse(CodeBlockType* codeBlock, const InstructionStream& instructions, BytecodeGraph&, BytecodeIndex bytecodeIndex, const UseFunctor& use)
+{
+    auto* instruction = instructions.at(bytecodeIndex).ptr();
     computeUsesForBytecodeIndex(
-        codeBlock, opcodeID, instruction,
+        codeBlock, instruction,
         [&] (VirtualRegister operand) {
             if (isValidRegisterForLiveness(operand))
                 use(operand.toLocal());
         });
+}
 
+template<typename CodeBlockType, typename UseFunctor>
+inline void BytecodeLivenessPropagation::stepOverInstructionUseInExceptionHandler(CodeBlockType* codeBlock, const InstructionStream&, BytecodeGraph& graph, BytecodeIndex bytecodeIndex, const UseFunctor& use)
+{
     // If we have an exception handler, we want the live-in variables of the 
     // exception handler block to be included in the live-in of this particular bytecode.
     if (auto* handler = codeBlock->handlerForBytecodeIndex(bytecodeIndex)) {
@@ -104,6 +94,31 @@ inline void BytecodeLivenessPropagation::stepOverInstruction(CodeBlockType* code
     }
 }
 
+// Simplified interface to bytecode use/def, which determines defs first and then uses, and includes
+// exception handlers in the uses.
+template<typename CodeBlockType, typename UseFunctor, typename DefFunctor>
+inline void BytecodeLivenessPropagation::stepOverInstruction(CodeBlockType* codeBlock, const InstructionStream& instructions, BytecodeGraph& graph, BytecodeIndex bytecodeIndex, const UseFunctor& use, const DefFunctor& def)
+{
+    // This abstractly execute the instruction in reverse. Instructions logically first use operands and
+    // then define operands. This logical ordering is necessary for operations that use and def the same
+    // operand, like:
+    //
+    //     op_add loc1, loc1, loc2
+    //
+    // The use of loc1 happens before the def of loc1. That's a semantic requirement since the add
+    // operation cannot travel forward in time to read the value that it will produce after reading that
+    // value. Since we are executing in reverse, this means that we must do defs before uses (reverse of
+    // uses before defs).
+    //
+    // Since this is a liveness analysis, this ordering ends up being particularly important: if we did
+    // uses before defs, then the add operation above would appear to not have loc1 live, since we'd
+    // first add it to the out set (the use), and then we'd remove it (the def).
+
+    stepOverInstructionDef(codeBlock, instructions, graph, bytecodeIndex, def);
+    stepOverInstructionUseInExceptionHandler(codeBlock, instructions, graph, bytecodeIndex, use);
+    stepOverInstructionUse(codeBlock, instructions, graph, bytecodeIndex, use);
+}
+
 template<typename CodeBlockType>
 inline void BytecodeLivenessPropagation::stepOverInstruction(CodeBlockType* codeBlock, const InstructionStream& instructions, BytecodeGraph& graph, BytecodeIndex bytecodeIndex, FastBitVector& out)
 {
index c059940..28fdb38 100644 (file)
@@ -46,8 +46,9 @@ namespace JSC {
 #define DEFS USES_OR_DEFS
 
 template<typename Block, typename Functor>
-void computeUsesForBytecodeIndex(Block* codeBlock, OpcodeID opcodeID, const Instruction* instruction, const Functor& functor)
+void computeUsesForBytecodeIndex(Block* codeBlock, const Instruction* instruction, const Functor& functor)
 {
+    OpcodeID opcodeID = instruction->opcodeID();
     if (opcodeID != op_enter && (codeBlock->wasCompiledWithDebuggingOpcodes() || codeBlock->usesEval()) && codeBlock->scopeRegister().isValid())
         functor(codeBlock->scopeRegister());
 
@@ -298,9 +299,9 @@ void computeUsesForBytecodeIndex(Block* codeBlock, OpcodeID opcodeID, const Inst
 }
 
 template<typename Block, typename Functor>
-void computeDefsForBytecodeIndex(Block* codeBlock, OpcodeID opcodeID, const Instruction* instruction, const Functor& functor)
+void computeDefsForBytecodeIndex(Block* codeBlock, const Instruction* instruction, const Functor& functor)
 {
-    switch (opcodeID) {
+    switch (instruction->opcodeID()) {
     case op_wide16:
     case op_wide32:
         RELEASE_ASSERT_NOT_REACHED();
index 9a88437..a7fa338 100644 (file)
 
 #pragma once
 
+#include "BytecodeLivenessAnalysis.h"
 #include <wtf/FastBitVector.h>
 
 namespace JSC {
 
 class BytecodeLivenessAnalysis;
 
-typedef HashMap<BytecodeIndex, FastBitVector> BytecodeToBitmapMap;
-
 class FullBytecodeLiveness {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    const FastBitVector& getLiveness(BytecodeIndex bytecodeIndex) const
+    const FastBitVector& getLiveness(BytecodeIndex bytecodeIndex, LivenessCalculationPoint point) const
     {
         // FIXME: What should this do when we have checkpoints?
-        return m_map[bytecodeIndex.offset()];
+        switch (point) {
+        case LivenessCalculationPoint::BeforeUse:
+            return m_beforeUseVector[bytecodeIndex.offset()];
+        case LivenessCalculationPoint::AfterUse:
+            return m_afterUseVector[bytecodeIndex.offset()];
+        }
+        ASSERT_NOT_REACHED();
     }
     
-    bool operandIsLive(int operand, BytecodeIndex bytecodeIndex) const
+    bool operandIsLive(int operand, BytecodeIndex bytecodeIndex, LivenessCalculationPoint point) const
     {
-        return operandIsAlwaysLive(operand) || operandThatIsNotAlwaysLiveIsLive(getLiveness(bytecodeIndex), operand);
+        return operandIsAlwaysLive(operand) || operandThatIsNotAlwaysLiveIsLive(getLiveness(bytecodeIndex, point), operand);
     }
     
 private:
     friend class BytecodeLivenessAnalysis;
     
-    Vector<FastBitVector, 0, UnsafeVectorOverflow> m_map;
+    // FIXME: Use FastBitVector's view mechansim to make them compact.
+    // https://bugs.webkit.org/show_bug.cgi?id=204427<Paste>
+    Vector<FastBitVector, 0, UnsafeVectorOverflow> m_beforeUseVector;
+    Vector<FastBitVector, 0, UnsafeVectorOverflow> m_afterUseVector;
 };
 
 } // namespace JSC
index 164696b..cbdf269 100644 (file)
@@ -4891,10 +4891,8 @@ void ForInContext::finalize(BytecodeGenerator& generator, UnlinkedCodeBlock* cod
 
     for (unsigned offset = bodyBytecodeStartOffset(); isValid() && offset < bodyBytecodeEndOffset;) {
         auto instruction = generator.instructions().at(offset);
-        OpcodeID opcodeID = instruction->opcodeID();
-
-        ASSERT(opcodeID != op_enter);
-        computeDefsForBytecodeIndex(codeBlock, opcodeID, instruction.ptr(), [&] (VirtualRegister operand) {
+        ASSERT(!instruction->is<OpEnter>());
+        computeDefsForBytecodeIndex(codeBlock, instruction.ptr(), [&] (VirtualRegister operand) {
             if (local()->virtualRegister() == operand)
                 invalidate();
         });
index aaeab19..0ba20f1 100644 (file)
@@ -33,6 +33,7 @@
 #include "BasicBlockLocation.h"
 #include "BuiltinNames.h"
 #include "BytecodeGenerator.h"
+#include "BytecodeUseDef.h"
 #include "CallLinkStatus.h"
 #include "CodeBlock.h"
 #include "CodeBlockWithJITType.h"
@@ -74,6 +75,7 @@
 #include <wtf/CommaPrinter.h>
 #include <wtf/HashMap.h>
 #include <wtf/MathExtras.h>
+#include <wtf/Scope.h>
 #include <wtf/SetForScope.h>
 #include <wtf/StdLibExtras.h>
 
@@ -565,6 +567,7 @@ private:
     template<typename AddFlushDirectFunc, typename AddPhantomLocalDirectFunc>
     void flushForTerminalImpl(CodeOrigin origin, const AddFlushDirectFunc& addFlushDirect, const AddPhantomLocalDirectFunc& addPhantomLocalDirect)
     {
+        bool isCallerOrigin = false;
         origin.walkUpInlineStack(
             [&] (CodeOrigin origin) {
                 BytecodeIndex bytecodeIndex = origin.bytecodeIndex();
@@ -573,12 +576,12 @@ private:
 
                 CodeBlock* codeBlock = m_graph.baselineCodeBlockFor(inlineCallFrame);
                 FullBytecodeLiveness& fullLiveness = m_graph.livenessFor(codeBlock);
-                const FastBitVector& livenessAtBytecode = fullLiveness.getLiveness(bytecodeIndex);
-
+                const auto& livenessAtBytecode = fullLiveness.getLiveness(bytecodeIndex, m_graph.appropriateLivenessCalculationPoint(origin, isCallerOrigin));
                 for (unsigned local = codeBlock->numCalleeLocals(); local--;) {
                     if (livenessAtBytecode[local])
                         addPhantomLocalDirect(inlineCallFrame, remapOperand(inlineCallFrame, virtualRegisterForLocal(local)));
                 }
+                isCallerOrigin = true;
             });
     }
 
index c37510c..0bf1c3f 100644 (file)
@@ -73,8 +73,8 @@ void forAllKilledOperands(Graph& graph, Node* nodeBefore, Node* nodeAfter, const
         int stackOffset = beforeInlineCallFrame ? beforeInlineCallFrame->stackOffset : 0;
         CodeBlock* codeBlock = graph.baselineCodeBlockFor(beforeInlineCallFrame);
         FullBytecodeLiveness& fullLiveness = graph.livenessFor(codeBlock);
-        const FastBitVector& liveBefore = fullLiveness.getLiveness(before.bytecodeIndex());
-        const FastBitVector& liveAfter = fullLiveness.getLiveness(after.bytecodeIndex());
+        const FastBitVector& liveBefore = fullLiveness.getLiveness(before.bytecodeIndex(), LivenessCalculationPoint::BeforeUse);
+        const FastBitVector& liveAfter = fullLiveness.getLiveness(after.bytecodeIndex(), LivenessCalculationPoint::BeforeUse);
         
         (liveBefore & ~liveAfter).forEachSetBit(
             [&] (size_t relativeLocal) {
index 2675932..f874f33 100644 (file)
@@ -1139,6 +1139,7 @@ bool Graph::isLiveInBytecode(VirtualRegister operand, CodeOrigin codeOrigin)
     
     if (verbose)
         dataLog("Checking of operand is live: ", operand, "\n");
+    bool isCallerOrigin = false;
     CodeOrigin* codeOriginPtr = &codeOrigin;
     for (;;) {
         VirtualRegister reg = VirtualRegister(
@@ -1172,7 +1173,10 @@ bool Graph::isLiveInBytecode(VirtualRegister operand, CodeOrigin codeOrigin)
 
             if (verbose)
                 dataLog("Asking the bytecode liveness.\n");
-            return livenessFor(inlineCallFrame).operandIsLive(reg.offset(), codeOriginPtr->bytecodeIndex());
+            CodeBlock* codeBlock = baselineCodeBlockFor(inlineCallFrame);
+            FullBytecodeLiveness& fullLiveness = livenessFor(codeBlock);
+            BytecodeIndex bytecodeIndex = codeOriginPtr->bytecodeIndex();
+            return fullLiveness.operandIsLive(reg.offset(), bytecodeIndex, appropriateLivenessCalculationPoint(*codeOriginPtr, isCallerOrigin));
         }
 
         if (!inlineCallFrame) {
@@ -1193,6 +1197,7 @@ bool Graph::isLiveInBytecode(VirtualRegister operand, CodeOrigin codeOrigin)
         // We need to handle tail callers because we may decide to exit to the
         // the return bytecode following the tail call.
         codeOriginPtr = &inlineCallFrame->directCaller;
+        isCallerOrigin = true;
     }
     
     RELEASE_ASSERT_NOT_REACHED();
index 869faee..fab9cf0 100644 (file)
@@ -856,6 +856,7 @@ public:
 
         CodeOrigin* codeOriginPtr = &codeOrigin;
         
+        bool isCallerOrigin = false;
         for (;;) {
             InlineCallFrame* inlineCallFrame = codeOriginPtr->inlineCallFrame();
             VirtualRegister stackOffset(inlineCallFrame ? inlineCallFrame->stackOffset : 0);
@@ -869,7 +870,7 @@ public:
             
             CodeBlock* codeBlock = baselineCodeBlockFor(inlineCallFrame);
             FullBytecodeLiveness& fullLiveness = livenessFor(codeBlock);
-            const FastBitVector& liveness = fullLiveness.getLiveness(codeOriginPtr->bytecodeIndex());
+            const auto& livenessAtBytecode = fullLiveness.getLiveness(codeOriginPtr->bytecodeIndex(), appropriateLivenessCalculationPoint(*codeOriginPtr, isCallerOrigin));
             for (unsigned relativeLocal = codeBlock->numCalleeLocals(); relativeLocal--;) {
                 VirtualRegister reg = stackOffset + virtualRegisterForLocal(relativeLocal);
                 
@@ -877,7 +878,7 @@ public:
                 if (reg >= exclusionStart && reg < exclusionEnd)
                     continue;
                 
-                if (liveness[relativeLocal])
+                if (livenessAtBytecode[relativeLocal])
                     functor(reg);
             }
             
@@ -899,12 +900,43 @@ public:
             // We need to handle tail callers because we may decide to exit to the
             // the return bytecode following the tail call.
             codeOriginPtr = &inlineCallFrame->directCaller;
+            isCallerOrigin = true;
         }
     }
     
     // Get a BitVector of all of the non-argument locals live right now. This is mostly useful if
     // you want to compare two sets of live locals from two different CodeOrigins.
     BitVector localsLiveInBytecode(CodeOrigin);
+
+    LivenessCalculationPoint appropriateLivenessCalculationPoint(CodeOrigin origin, bool isCallerOrigin)
+    {
+        if (isCallerOrigin) {
+            // We do not need to keep used registers of call bytecodes live when terminating in inlined function,
+            // except for inlining invoked by non call bytecodes including getter/setter calls.
+            BytecodeIndex bytecodeIndex = origin.bytecodeIndex();
+            InlineCallFrame* inlineCallFrame = origin.inlineCallFrame();
+            CodeBlock* codeBlock = baselineCodeBlockFor(inlineCallFrame);
+            auto instruction = codeBlock->instructions().at(bytecodeIndex.offset());
+            switch (instruction->opcodeID()) {
+            case op_call_varargs:
+            case op_tail_call_varargs:
+            case op_construct_varargs:
+                // When inlining varargs call, uses include array used for varargs. But when we are in inlined function,
+                // the content of this is already read and flushed to the stack. So, at this point, we no longer need to
+                // keep these use registers. We can use the liveness at LivenessCalculationPoint::AfterUse point.
+                // This is important to kill arguments allocations in DFG (not in FTL) when calling a function in a
+                // `func.apply(undefined, arguments)` manner.
+                return LivenessCalculationPoint::AfterUse;
+            default:
+                // We could list up the other bytecodes here, like, `op_call`, `op_get_by_id` (getter inlining). But we don't do that.
+                // To list up bytecodes here, we must ensure that these bytecodes never use `uses` registers after inlining. So we cannot
+                // return LivenessCalculationPoint::AfterUse blindly if isCallerOrigin = true. And since excluding liveness in the other
+                // bytecodes does not offer practical benefit, we do not try it.
+                break;
+            }
+        }
+        return LivenessCalculationPoint::BeforeUse;
+    }
     
     // Tells you all of the arguments and locals live at the given CodeOrigin. This is a small
     // extension to forAllLocalsLiveInBytecode(), since all arguments are always presumed live.
index 514b3eb..a258497 100644 (file)
@@ -91,6 +91,7 @@ macro makeReturnProfiled(opcodeStruct, get, metadata, dispatch, fn)
 end
 
 
+# After calling, calling bytecode is claiming input registers are not used.
 macro dispatchAfterCall(size, opcodeStruct, dispatch)
     loadi ArgumentCount + TagOffset[cfr], PC
     get(size, opcodeStruct, m_dst, t3)
index 713039a..3dd2eb4 100644 (file)
@@ -90,6 +90,7 @@ macro valueProfile(opcodeStruct, metadata, value)
     storeq value, %opcodeStruct%::Metadata::m_profile.m_buckets[metadata]
 end
 
+# After calling, calling bytecode is claiming input registers are not used.
 macro dispatchAfterCall(size, opcodeStruct, dispatch)
     loadi ArgumentCount + TagOffset[cfr], PC
     loadp CodeBlock[cfr], PB