Changeset 264049 in webkit for trunk/Source/JavaScriptCore/bytecode/BytecodeUseDef.cpp

Timestamp:

Jul 7, 2020, 5:32:35 PM (5 years ago)

Author:

[email protected]

Message:

Bytecode UseDef should be aware of checkpoints
​https://bugs.webkit.org/show_bug.cgi?id=213566

Reviewed by Saam Barati.

JSTests:

• stress/def-then-use-in-single-bytecode-with-checkpoints-for-of.js: Added.

(foo):

Source/JavaScriptCore:

Previously, we tried to solve teaching DFG about uses and defs of
locals across checkpoints by asking what locals were def'd at some
checkpoint. However, this was subtly wrong because we couldn't
report any uses at subsequent checkpoints so DFG thought the
local was dead immediately after its birth.

This patch reverts that change and instead teaches BytecodeUseDef
about checkpoints. Right now, BytecodeUseDef only knows about
locals at checkpoints but in the future we may teach it about tmps
at well.

Since the vectors containing our liveness bitmaps were already
sparse (they are indexed by the bytecode offset) we can reuse the
gaps to hold our checkpoint liveness information. To make sure we
don't overlap between the next bytecode and a checkpoint for the
current bytecode there is now a static assert that the length of
the bytecode is greater than the number of checkpoints. This
assumption is already true for existing bytecodes with checkpoints (and
likely to be true for future ones anyway).

Many of the BytecodeLivenessPropegation functions have been
renamed to reflect that they operate over the full instruction,
including checkpoints, rather than just the BytecodeIndex passed.

Lastly, this patch makes a speculative fix to forAllKilledOperands where we
wouldn't report that all tmps die at the end of each bytecode. I can't think
of a case where this would break things but it's probably good hygiene.

• bytecode/BytecodeGeneratorification.cpp:

(JSC::GeneratorLivenessAnalysis::run):

• bytecode/BytecodeIndex.h:

(JSC::BytecodeIndex::BytecodeIndex):
(JSC::BytecodeIndex::checkpoint const):
(JSC::BytecodeIndex::withCheckpoint const):
(JSC::BytecodeIndex::pack):

• bytecode/BytecodeLivenessAnalysis.cpp:

(JSC::BytecodeLivenessAnalysis::computeFullLiveness):
(JSC::BytecodeLivenessAnalysis::dumpResults):
(JSC::tmpLivenessForCheckpoint):
(JSC::BytecodeLivenessAnalysis::getLivenessInfoAtBytecodeIndex): Deleted.
(JSC::livenessForCheckpoint): Deleted.

• bytecode/BytecodeLivenessAnalysis.h:

(JSC::BytecodeLivenessAnalysis::getLivenessInfoAtInstruction):

• bytecode/BytecodeLivenessAnalysisInlines.h:

(JSC::BytecodeLivenessPropagation::stepOverBytecodeIndexDef):
(JSC::BytecodeLivenessPropagation::stepOverBytecodeIndexUse):
(JSC::BytecodeLivenessPropagation::stepOverBytecodeIndexUseInExceptionHandler):
(JSC::BytecodeLivenessPropagation::stepOverBytecodeIndex):
(JSC::BytecodeLivenessPropagation::stepOverInstruction):
(JSC::BytecodeLivenessPropagation::computeLocalLivenessForInstruction):
(JSC::BytecodeLivenessPropagation::computeLocalLivenessForBlock):
(JSC::BytecodeLivenessPropagation::getLivenessInfoAtInstruction):
(JSC::BytecodeLivenessPropagation::stepOverInstructionDef): Deleted.
(JSC::BytecodeLivenessPropagation::stepOverInstructionUse): Deleted.
(JSC::BytecodeLivenessPropagation::stepOverInstructionUseInExceptionHandler): Deleted.
(JSC::BytecodeLivenessPropagation::computeLocalLivenessForBytecodeIndex): Deleted.
(JSC::BytecodeLivenessPropagation::getLivenessInfoAtBytecodeIndex): Deleted.

• bytecode/BytecodeUseDef.cpp:

(JSC::computeUsesForBytecodeIndexImpl):
(JSC::computeDefsForBytecodeIndexImpl):

• bytecode/BytecodeUseDef.h:

(JSC::computeUsesForBytecodeIndex):
(JSC::computeDefsForBytecodeIndex):

• bytecode/CodeBlock.cpp:

(JSC::CodeBlock::ensureCatchLivenessIsComputedForBytecodeIndexSlow):
(JSC::CodeBlock::validate):

• bytecode/FullBytecodeLiveness.h:

(JSC::FullBytecodeLiveness::getLiveness const):
(JSC::FullBytecodeLiveness::toIndex):

• bytecode/Instruction.h:

(JSC::BaseInstruction::numberOfCheckpoints const):

• bytecompiler/BytecodeGenerator.cpp:

(JSC::ForInContext::finalize):

• dfg/DFGForAllKills.h:

(JSC::DFG::forAllKilledOperands):

• dfg/DFGGraph.cpp:

(JSC::DFG::Graph::isLiveInBytecode):

• dfg/DFGGraph.h:
• dfg/DFGMovHintRemovalPhase.cpp:
• dfg/DFGPlan.cpp:

(JSC::DFG::Plan::cleanMustHandleValuesIfNecessary):

• generator/Opcode.rb:
• generator/Section.rb:

File:

• trunk/Source/JavaScriptCore/bytecode/BytecodeUseDef.cpp (modified) (9 diffs)

trunk/Source/JavaScriptCore/bytecode/BytecodeUseDef.cpp

@@ -34 +34 @@
 #define USES_OR_DEFS(__opcode, ...) \
     case __opcode::opcodeID: { \
+        static_assert(__opcode::opcodeID >= NUMBER_OF_BYTECODE_WITH_CHECKPOINTS, "Don't use this macro for bytecodes that have checkpoints."); \
         auto __bytecode = instruction->as<__opcode>(); \
         WTF_LAZY_FOR_EACH_TERM(CALL_FUNCTOR, __VA_ARGS__) \
@@ -42 +43 @@
 #define DEFS USES_OR_DEFS
 
-void computeUsesForBytecodeIndexImpl(VirtualRegister scopeRegister, const Instruction* instruction, const Function<void(VirtualRegister)>& functor)
+void computeUsesForBytecodeIndexImpl(VirtualRegister scopeRegister, const Instruction* instruction, Checkpoint checkpoint, const ScopedLambda<void(VirtualRegister)>& functor)
 {
     OpcodeID opcodeID = instruction->opcodeID();
 
-    auto handleNewArrayLike = [&](auto op) {
+
+    auto handleNewArrayLike = [&] (auto op) {
         int base = op.m_argv.offset();
         for (int i = 0; i < static_cast<int>(op.m_argc); i++)
@@ -52 +54 @@
     };
 
-    auto handleOpCallLike = [&](auto op) {
+    auto handleOpCallLike = [&] (auto op) {
         functor(op.m_callee);
         int lastArg = -static_cast<int>(op.m_argv) + CallFrame::thisArgumentOffset();
@@ -60 +62 @@
             functor(scopeRegister);
         return;
+    };
+
+    auto useAtEachCheckpoint = [&] (auto... virtualRegisters) {
+        (functor(virtualRegisters), ...);
+    };
+
+    auto useAtEachCheckpointStartingWith = [&] (Checkpoint firstUse, auto... virtualRegisters) {
+        if (checkpoint >= firstUse)
+            (functor(virtualRegisters), ...);
     };
 
@@ -251 +262 @@
     USES(OpHasOwnStructureProperty, base, property, enumerator)
     USES(OpInStructureProperty, base, property, enumerator)
-    USES(OpConstructVarargs, callee, thisValue, arguments)
-    USES(OpCallVarargs, callee, thisValue, arguments)
-    USES(OpTailCallVarargs, callee, thisValue, arguments)
+
+    case op_call_varargs: {
+        auto bytecode = instruction->as<OpCallVarargs>();
+        useAtEachCheckpoint(bytecode.m_callee, bytecode.m_thisValue, bytecode.m_arguments);
+        return;
+    }
+    case op_tail_call_varargs: {
+        auto bytecode = instruction->as<OpTailCallVarargs>();
+        useAtEachCheckpoint(bytecode.m_callee, bytecode.m_thisValue, bytecode.m_arguments);
+        return;
+    }
+    case op_construct_varargs: {
+        auto bytecode = instruction->as<OpConstructVarargs>();
+        useAtEachCheckpoint(bytecode.m_callee, bytecode.m_thisValue, bytecode.m_arguments);
+        return;
+    }
 
     USES(OpGetDirectPname, base, property, index, enumerator)
@@ -266 +290 @@
     USES(OpYield, generator, argument)
 
-    USES(OpIteratorOpen, symbolIterator, iterable)
-    USES(OpIteratorNext, iterator, next, iterable)
+    case op_iterator_open: {
+        auto bytecode = instruction->as<OpIteratorOpen>();
+        useAtEachCheckpointStartingWith(OpIteratorOpen::symbolCall, bytecode.m_symbolIterator, bytecode.m_iterable);
+        useAtEachCheckpointStartingWith(OpIteratorOpen::getNext, bytecode.m_iterator);
+        return;
+    }
+
+    case op_iterator_next: {
+        auto bytecode = instruction->as<OpIteratorNext>();
+        useAtEachCheckpoint(bytecode.m_iterator);
+        useAtEachCheckpointStartingWith(OpIteratorNext::computeNext, bytecode.m_next, bytecode.m_iterable);
+        return;
+    }
 
     case op_new_array_with_spread:
@@ -303 +338 @@
 }
 
-void computeDefsForBytecodeIndexImpl(unsigned numVars, const Instruction* instruction, const Function<void(VirtualRegister)>& functor)
+void computeDefsForBytecodeIndexImpl(unsigned numVars, const Instruction* instruction, Checkpoint checkpoint, const ScopedLambda<void(VirtualRegister)>& functor)
 {
+
+    auto defAt = [&] (Checkpoint target, VirtualRegister operand) {
+        if (target == checkpoint)
+            functor(operand);
+    };
+
     switch (instruction->opcodeID()) {
     case op_wide16:
@@ -413 +454 @@
     DEFS(OpNewAsyncFunc, dst)
     DEFS(OpNewAsyncFuncExp, dst)
-    DEFS(OpCallVarargs, dst)
-    DEFS(OpTailCallVarargs, dst)
+    case op_call_varargs: {
+        auto bytecode = instruction->as<OpCallVarargs>();
+        defAt(OpCallVarargs::makeCall, bytecode.m_dst);
+        return;
+    }
+    case op_tail_call_varargs: {
+        auto bytecode = instruction->as<OpTailCallVarargs>();
+        defAt(OpTailCallVarargs::makeCall, bytecode.m_dst);
+        return;
+    }
+    case op_construct_varargs: {
+        auto bytecode = instruction->as<OpConstructVarargs>();
+        defAt(OpConstructVarargs::makeCall, bytecode.m_dst);
+        return;
+    }
+
     DEFS(OpTailCallForwardArguments, dst)
-    DEFS(OpConstructVarargs, dst)
     DEFS(OpGetFromScope, dst)
     DEFS(OpCall, dst)
@@ -502 +556 @@
     DEFS(OpCatch, exception, thrownValue)
 
-    DEFS(OpIteratorOpen, iterator, next)
-    DEFS(OpIteratorNext, done, value)
+    case op_iterator_open: {
+        auto bytecode = instruction->as<OpIteratorOpen>();
+
+        defAt(OpIteratorOpen::symbolCall, bytecode.m_iterator);
+        defAt(OpIteratorOpen::getNext, bytecode.m_next);
+        return;
+    }
+
+    case op_iterator_next: {
+        auto bytecode = instruction->as<OpIteratorNext>();
+
+        defAt(OpIteratorNext::getDone, bytecode.m_done);
+        // We need to claim we set m_value here because we could early exit from the bytecode if we are done.
+        defAt(OpIteratorNext::getDone, bytecode.m_value);
+
+        defAt(OpIteratorNext::getValue, bytecode.m_value);
+        return;
+    }
 
     case op_enter: {