[Interpreter] Move dead code elimination to BytecodeArrayWriter.

Move dead bytecode elimination from a seperate bytecode pipeline optimizer
into the BytecodeArrayWriter. This removes the last bytecode pipeline
optimizer, which means we can remove the Bytecode pipeline which,
which should increase compile speed.

BUG=v8:6194

Change-Id: I47fb3c3463b2b8a92e02cf7a6b608683fcfa5261
Reviewed-on: https://chromium-review.googlesource.com/471407
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44568}

BUILD.gn

@@ -1620,8 +1620,6 @@ v8_source_set("v8_base") {
     "src/interpreter/bytecode-array-random-iterator.h",
     "src/interpreter/bytecode-array-writer.cc",
     "src/interpreter/bytecode-array-writer.h",
-    "src/interpreter/bytecode-dead-code-optimizer.cc",
-    "src/interpreter/bytecode-dead-code-optimizer.h",
     "src/interpreter/bytecode-decoder.cc",
     "src/interpreter/bytecode-decoder.h",
     "src/interpreter/bytecode-flags.cc",

src/flag-definitions.h

@@ -309,8 +309,6 @@ DEFINE_BOOL(string_slices, true, "use string slices")
 
 // Flags for Ignition.
 DEFINE_BOOL(ignition, false, "use ignition interpreter")
-DEFINE_BOOL(ignition_deadcode, true,
-            "use ignition dead code elimination optimizer")
 DEFINE_BOOL(ignition_osr, true, "enable support for OSR from ignition code")
 DEFINE_BOOL(ignition_elide_noneffectful_bytecodes, true,
             "elide bytecodes which won't have any external effect")

src/interpreter/bytecode-array-builder.cc

@@ -6,7 +6,6 @@
 
 #include "src/globals.h"
 #include "src/interpreter/bytecode-array-writer.h"
-#include "src/interpreter/bytecode-dead-code-optimizer.h"
 #include "src/interpreter/bytecode-label.h"
 #include "src/interpreter/bytecode-register-optimizer.h"
 #include "src/interpreter/interpreter-intrinsics.h"
@@ -57,10 +56,6 @@ BytecodeArrayBuilder::BytecodeArrayBuilder(
   DCHECK_GE(context_register_count_, 0);
   DCHECK_GE(local_register_count_, 0);
 
-  if (FLAG_ignition_deadcode) {
-    pipeline_ = new (zone) BytecodeDeadCodeOptimizer(pipeline_);
-  }
-
   if (FLAG_ignition_reo) {
     register_optimizer_ = new (zone) BytecodeRegisterOptimizer(
         zone, &register_allocator_, fixed_register_count(), parameter_count,

src/interpreter/bytecode-array-writer.cc

@@ -28,8 +28,8 @@ BytecodeArrayWriter::BytecodeArrayWriter(
       last_bytecode_(Bytecode::kIllegal),
       last_bytecode_offset_(0),
       last_bytecode_had_source_info_(false),
-      elide_noneffectful_bytecodes_(
-          FLAG_ignition_elide_noneffectful_bytecodes) {
+      elide_noneffectful_bytecodes_(FLAG_ignition_elide_noneffectful_bytecodes),
+      exit_seen_in_block_(false) {
   bytecodes_.reserve(512);  // Derived via experimentation.
 }
 
@@ -60,7 +60,11 @@ Handle<BytecodeArray> BytecodeArrayWriter::ToBytecodeArray(
 // override
 void BytecodeArrayWriter::Write(BytecodeNode* node) {
   DCHECK(!Bytecodes::IsJump(node->bytecode()));
+
+  if (exit_seen_in_block_) return;  // Don't emit dead code.
+  UpdateExitSeenInBlock(node->bytecode());
   MaybeElideLastBytecode(node->bytecode(), node->source_info().is_valid());
+
   UpdateSourcePositionTable(node);
   EmitBytecode(node);
 }
@@ -68,7 +72,13 @@ void BytecodeArrayWriter::Write(BytecodeNode* node) {
 // override
 void BytecodeArrayWriter::WriteJump(BytecodeNode* node, BytecodeLabel* label) {
   DCHECK(Bytecodes::IsJump(node->bytecode()));
+
+  // TODO(rmcilroy): For forward jumps we could also mark the label as dead,
+  // thereby avoiding emitting dead code when we bind the label.
+  if (exit_seen_in_block_) return;  // Don't emit dead code.
+  UpdateExitSeenInBlock(node->bytecode());
   MaybeElideLastBytecode(node->bytecode(), node->source_info().is_valid());
+
   UpdateSourcePositionTable(node);
   EmitJump(node, label);
 }
@@ -83,6 +93,7 @@ void BytecodeArrayWriter::BindLabel(BytecodeLabel* label) {
   }
   label->bind_to(current_offset);
   InvalidateLastBytecode();
+  exit_seen_in_block_ = false;  // Starting a new basic block.
 }
 
 // override
@@ -97,6 +108,8 @@ void BytecodeArrayWriter::BindLabel(const BytecodeLabel& target,
   }
   label->bind_to(target.offset());
   InvalidateLastBytecode();
+  // exit_seen_in_block_ was reset when target was bound, so shouldn't be
+  // changed here.
 }
 
 void BytecodeArrayWriter::UpdateSourcePositionTable(
@@ -110,6 +123,20 @@ void BytecodeArrayWriter::UpdateSourcePositionTable(
   }
 }
 
+void BytecodeArrayWriter::UpdateExitSeenInBlock(Bytecode bytecode) {
+  switch (bytecode) {
+    case Bytecode::kReturn:
+    case Bytecode::kThrow:
+    case Bytecode::kReThrow:
+    case Bytecode::kJump:
+    case Bytecode::kJumpConstant:
+      exit_seen_in_block_ = true;
+      break;
+    default:
+      break;
+  }
+}
+
 void BytecodeArrayWriter::MaybeElideLastBytecode(Bytecode next_bytecode,
                                                  bool has_source_info) {
   if (!elide_noneffectful_bytecodes_) return;

src/interpreter/bytecode-array-writer.h

@@ -65,6 +65,8 @@ class V8_EXPORT_PRIVATE BytecodeArrayWriter final
   void EmitJump(BytecodeNode* node, BytecodeLabel* label);
   void UpdateSourcePositionTable(const BytecodeNode* const node);
 
+  void UpdateExitSeenInBlock(Bytecode bytecode);
+
   void MaybeElideLastBytecode(Bytecode next_bytecode, bool has_source_info);
   void InvalidateLastBytecode();
 
@@ -86,6 +88,8 @@ class V8_EXPORT_PRIVATE BytecodeArrayWriter final
   bool last_bytecode_had_source_info_;
   bool elide_noneffectful_bytecodes_;
 
+  bool exit_seen_in_block_;
+
   friend class BytecodeArrayWriterUnittest;
   DISALLOW_COPY_AND_ASSIGN(BytecodeArrayWriter);
 };

src/interpreter/bytecode-dead-code-optimizer.cc

-// Copyright 2016 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "src/interpreter/bytecode-dead-code-optimizer.h"
-
-namespace v8 {
-namespace internal {
-namespace interpreter {
-
-BytecodeDeadCodeOptimizer::BytecodeDeadCodeOptimizer(
-    BytecodePipelineStage* next_stage)
-    : next_stage_(next_stage), exit_seen_in_block_(false) {}
-
-// override
-Handle<BytecodeArray> BytecodeDeadCodeOptimizer::ToBytecodeArray(
-    Isolate* isolate, int register_count, int parameter_count,
-    Handle<FixedArray> handler_table) {
-  return next_stage_->ToBytecodeArray(isolate, register_count, parameter_count,
-                                      handler_table);
-}
-
-// override
-void BytecodeDeadCodeOptimizer::Write(BytecodeNode* node) {
-  // Don't emit dead code.
-  if (exit_seen_in_block_) return;
-
-  switch (node->bytecode()) {
-    case Bytecode::kReturn:
-    case Bytecode::kThrow:
-    case Bytecode::kReThrow:
-      exit_seen_in_block_ = true;
-      break;
-    default:
-      break;
-  }
-
-  next_stage_->Write(node);
-}
-
-// override
-void BytecodeDeadCodeOptimizer::WriteJump(BytecodeNode* node,
-                                          BytecodeLabel* label) {
-  // Don't emit dead code.
-  // TODO(rmcilroy): For forward jumps we could mark the label as dead, thereby
-  // avoiding emitting dead code when we bind the label.
-  if (exit_seen_in_block_) return;
-
-  switch (node->bytecode()) {
-    case Bytecode::kJump:
-    case Bytecode::kJumpConstant:
-      exit_seen_in_block_ = true;
-      break;
-    default:
-      break;
-  }
-
-  next_stage_->WriteJump(node, label);
-}
-
-// override
-void BytecodeDeadCodeOptimizer::BindLabel(BytecodeLabel* label) {
-  next_stage_->BindLabel(label);
-  exit_seen_in_block_ = false;
-}
-
-// override
-void BytecodeDeadCodeOptimizer::BindLabel(const BytecodeLabel& target,
-                                          BytecodeLabel* label) {
-  next_stage_->BindLabel(target, label);
-  // exit_seen_in_block_ was reset when target was bound, so shouldn't be
-  // changed here.
-}
-
-}  // namespace interpreter
-}  // namespace internal
-}  // namespace v8

src/interpreter/bytecode-dead-code-optimizer.h

-// Copyright 2016 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef V8_INTERPRETER_BYTECODE_DEAD_CODE_OPTIMIZER_H_
-#define V8_INTERPRETER_BYTECODE_DEAD_CODE_OPTIMIZER_H_
-
-#include "src/base/compiler-specific.h"
-#include "src/globals.h"
-#include "src/interpreter/bytecode-pipeline.h"
-
-namespace v8 {
-namespace internal {
-namespace interpreter {
-
-// An optimization stage for eliminating obviously dead code in bytecode
-// generation.
-class V8_EXPORT_PRIVATE BytecodeDeadCodeOptimizer final
-    : public NON_EXPORTED_BASE(BytecodePipelineStage),
-      public NON_EXPORTED_BASE(ZoneObject) {
- public:
-  explicit BytecodeDeadCodeOptimizer(BytecodePipelineStage* next_stage);
-
-  // BytecodePipelineStage interface.
-  void Write(BytecodeNode* node) override;
-  void WriteJump(BytecodeNode* node, BytecodeLabel* label) override;
-  void BindLabel(BytecodeLabel* label) override;
-  void BindLabel(const BytecodeLabel& target, BytecodeLabel* label) override;
-  Handle<BytecodeArray> ToBytecodeArray(
-      Isolate* isolate, int register_count, int parameter_count,
-      Handle<FixedArray> handler_table) override;
-
- private:
-  BytecodePipelineStage* next_stage_;
-  bool exit_seen_in_block_;
-
-  DISALLOW_COPY_AND_ASSIGN(BytecodeDeadCodeOptimizer);
-};
-
-}  // namespace interpreter
-}  // namespace internal
-}  // namespace v8
-
-#endif  // V8_INTERPRETER_BYTECODE_DEAD_CODE_OPTIMIZER_H_

src/v8.gyp

@@ -1030,8 +1030,6 @@
         'interpreter/bytecode-array-random-iterator.h',
         'interpreter/bytecode-array-writer.cc',
         'interpreter/bytecode-array-writer.h',
-        'interpreter/bytecode-dead-code-optimizer.cc',
-        'interpreter/bytecode-dead-code-optimizer.h',
         'interpreter/bytecode-decoder.cc',
         'interpreter/bytecode-decoder.h',
         'interpreter/bytecode-flags.cc',

test/cctest/interpreter/test-source-positions.cc

@@ -19,11 +19,9 @@ namespace interpreter {
 
 // Flags enabling optimizations that change generated bytecode array.
 // Format is <command-line flag> <flag name> <bit index>
-#define OPTIMIZATION_FLAGS(V)                                                 \
-  V(FLAG_ignition_reo, kUseReo, 0)                                            \
-  V(FLAG_ignition_filter_expression_positions, kUseFilterExpressionPositions, \
-    2)                                                                        \
-  V(FLAG_ignition_deadcode, kUseDeadCode, 3)
+#define OPTIMIZATION_FLAGS(V)      \
+  V(FLAG_ignition_reo, kUseReo, 0) \
+  V(FLAG_ignition_filter_expression_positions, kUseFilterExpressionPositions, 2)
 
 #define DECLARE_BIT(_, Name, BitIndex) static const int Name = 1 << BitIndex;
 OPTIMIZATION_FLAGS(DECLARE_BIT)
@@ -34,13 +32,8 @@ OPTIMIZATION_FLAGS(DECLARE_BIT)
 // because it provides easier to comprehend failure case for humans.
 #define TEST_CASES(V)                                              \
   V(UsingReo, kUseReo)                                             \
-  V(UsingDeadCode, kUseDeadCode)                                   \
   V(UsingFilterExpressionPositions, kUseFilterExpressionPositions) \
-  V(UsingReoAndFilterExpressionPositions,                          \
-    kUseReo | kUseFilterExpressionPositions)                       \
-  V(UsingReoAndDeadCode, kUseReo | kUseDeadCode)                   \
-  V(UsingAllOptimizations,                                         \
-    kUseReo | kUseFilterExpressionPositions | kUseDeadCode)
+  V(UsingAllOptimizations, kUseReo | kUseFilterExpressionPositions)
 
 struct TestCaseData {
   TestCaseData(const char* const script,

test/unittests/BUILD.gn

@@ -115,7 +115,6 @@ v8_executable("unittests") {
     "interpreter/bytecode-array-iterator-unittest.cc",
     "interpreter/bytecode-array-random-iterator-unittest.cc",
     "interpreter/bytecode-array-writer-unittest.cc",
-    "interpreter/bytecode-dead-code-optimizer-unittest.cc",
     "interpreter/bytecode-decoder-unittest.cc",
     "interpreter/bytecode-operands-unittest.cc",
     "interpreter/bytecode-pipeline-unittest.cc",

test/unittests/interpreter/bytecode-array-writer-unittest.cc

@@ -293,6 +293,73 @@ TEST_F(BytecodeArrayWriterUnittest, ElideNoneffectfulBytecodes) {
   }
   CHECK(source_iterator.done());
 }
+
+TEST_F(BytecodeArrayWriterUnittest, DeadcodeElimination) {
+  static const uint8_t expected_bytes[] = {
+      // clang-format off
+      /*  0  10 E> */ B(StackCheck),
+      /*  1  55 S> */ B(LdaSmi), U8(127),
+      /*  3        */ B(Jump), U8(2),
+      /*  5  65 S> */ B(LdaSmi), U8(127),
+      /*  7        */ B(JumpIfFalse), U8(3),
+      /*  9  75 S> */ B(Return),
+      /*  10       */ B(JumpIfFalse), U8(3),
+      /*  12       */ B(Throw),
+      /*  13       */ B(JumpIfFalse), U8(3),
+      /*  15       */ B(ReThrow),
+      /*  16       */ B(Return),
+      // clang-format on
+  };
+
+  static const PositionTableEntry expected_positions[] = {
+      {0, 10, false}, {1, 55, true}, {5, 65, true}, {9, 75, true}};
+
+  BytecodeLabel after_jump, after_conditional_jump, after_return, after_throw,
+      after_rethrow;
+
+  Write(Bytecode::kStackCheck, {10, false});
+  Write(Bytecode::kLdaSmi, 127, {55, true});
+  WriteJump(Bytecode::kJump, &after_jump);
+  Write(Bytecode::kLdaSmi, 127);                               // Dead code.
+  WriteJump(Bytecode::kJumpIfFalse, &after_conditional_jump);  // Dead code.
+  writer()->BindLabel(&after_jump);
+  writer()->BindLabel(&after_conditional_jump);
+  Write(Bytecode::kLdaSmi, 127, {65, true});
+  WriteJump(Bytecode::kJumpIfFalse, &after_return);
+  Write(Bytecode::kReturn, {75, true});
+  Write(Bytecode::kLdaSmi, 127, {100, true});  // Dead code.
+  writer()->BindLabel(&after_return);
+  WriteJump(Bytecode::kJumpIfFalse, &after_throw);
+  Write(Bytecode::kThrow);
+  Write(Bytecode::kLdaSmi, 127);  // Dead code.
+  writer()->BindLabel(&after_throw);
+  WriteJump(Bytecode::kJumpIfFalse, &after_rethrow);
+  Write(Bytecode::kReThrow);
+  Write(Bytecode::kLdaSmi, 127);  // Dead code.
+  writer()->BindLabel(&after_rethrow);
+  Write(Bytecode::kReturn);
+
+  CHECK_EQ(bytecodes()->size(), arraysize(expected_bytes));
+  for (size_t i = 0; i < arraysize(expected_bytes); ++i) {
+    CHECK_EQ(static_cast<int>(bytecodes()->at(i)),
+             static_cast<int>(expected_bytes[i]));
+  }
+
+  Handle<BytecodeArray> bytecode_array = writer()->ToBytecodeArray(
+      isolate(), 0, 0, factory()->empty_fixed_array());
+  SourcePositionTableIterator source_iterator(
+      bytecode_array->source_position_table());
+  for (size_t i = 0; i < arraysize(expected_positions); ++i) {
+    const PositionTableEntry& expected = expected_positions[i];
+    CHECK_EQ(source_iterator.code_offset(), expected.code_offset);
+    CHECK_EQ(source_iterator.source_position().ScriptOffset(),
+             expected.source_position);
+    CHECK_EQ(source_iterator.is_statement(), expected.is_statement);
+    source_iterator.Advance();
+  }
+  CHECK(source_iterator.done());
+}
+
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8

test/unittests/interpreter/bytecode-dead-code-optimizer-unittest.cc

-// Copyright 2016 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "src/v8.h"
-
-#include "src/interpreter/bytecode-dead-code-optimizer.h"
-#include "src/interpreter/bytecode-label.h"
-#include "src/objects.h"
-#include "test/unittests/test-utils.h"
-
-namespace v8 {
-namespace internal {
-namespace interpreter {
-
-class BytecodeDeadCodeOptimizerTest : public BytecodePipelineStage,
-                                      public TestWithIsolateAndZone {
- public:
-  BytecodeDeadCodeOptimizerTest()
-      : dead_code_optimizer_(this), last_written_(Bytecode::kIllegal) {}
-  ~BytecodeDeadCodeOptimizerTest() override {}
-
-  void Write(BytecodeNode* node) override {
-    write_count_++;
-    last_written_ = *node;
-  }
-
-  void WriteJump(BytecodeNode* node, BytecodeLabel* label) override {
-    write_count_++;
-    last_written_ = *node;
-  }
-
-  void BindLabel(BytecodeLabel* label) override {}
-  void BindLabel(const BytecodeLabel& target, BytecodeLabel* label) override {}
-  Handle<BytecodeArray> ToBytecodeArray(
-      Isolate* isolate, int fixed_register_count, int parameter_count,
-      Handle<FixedArray> handle_table) override {
-    return Handle<BytecodeArray>();
-  }
-
-  BytecodeDeadCodeOptimizer* optimizer() { return &dead_code_optimizer_; }
-
-  int write_count() const { return write_count_; }
-  const BytecodeNode& last_written() const { return last_written_; }
-
- private:
-  BytecodeDeadCodeOptimizer dead_code_optimizer_;
-
-  int write_count_ = 0;
-  BytecodeNode last_written_;
-};
-
-TEST_F(BytecodeDeadCodeOptimizerTest, LiveCodeKept) {
-  BytecodeNode add(Bytecode::kAdd, Register(0).ToOperand(), 1);
-  optimizer()->Write(&add);
-  CHECK_EQ(write_count(), 1);
-  CHECK_EQ(add, last_written());
-
-  BytecodeLabel target;
-  BytecodeNode jump(Bytecode::kJump, 0);
-  optimizer()->WriteJump(&jump, &target);
-  CHECK_EQ(write_count(), 2);
-  CHECK_EQ(jump, last_written());
-}
-
-TEST_F(BytecodeDeadCodeOptimizerTest, DeadCodeAfterReturnEliminated) {
-  BytecodeNode ret(Bytecode::kReturn);
-  optimizer()->Write(&ret);
-  CHECK_EQ(write_count(), 1);
-  CHECK_EQ(ret, last_written());
-
-  BytecodeNode add(Bytecode::kAdd, Register(0).ToOperand(), 1);
-  optimizer()->Write(&add);
-  CHECK_EQ(write_count(), 1);
-  CHECK_EQ(ret, last_written());
-}
-
-TEST_F(BytecodeDeadCodeOptimizerTest, DeadCodeAfterThrowEliminated) {
-  BytecodeNode thrw(Bytecode::kThrow);
-  optimizer()->Write(&thrw);
-  CHECK_EQ(write_count(), 1);
-  CHECK_EQ(thrw, last_written());
-
-  BytecodeNode add(Bytecode::kAdd, Register(0).ToOperand(), 1);
-  optimizer()->Write(&add);
-  CHECK_EQ(write_count(), 1);
-  CHECK_EQ(thrw, last_written());
-}
-
-TEST_F(BytecodeDeadCodeOptimizerTest, DeadCodeAfterReThrowEliminated) {
-  BytecodeNode rethrow(Bytecode::kReThrow);
-  optimizer()->Write(&rethrow);
-  CHECK_EQ(write_count(), 1);
-  CHECK_EQ(rethrow, last_written());
-
-  BytecodeNode add(Bytecode::kAdd, Register(0).ToOperand(), 1);
-  optimizer()->Write(&add);
-  CHECK_EQ(write_count(), 1);
-  CHECK_EQ(rethrow, last_written());
-}
-
-TEST_F(BytecodeDeadCodeOptimizerTest, DeadCodeAfterJumpEliminated) {
-  BytecodeLabel target;
-  BytecodeNode jump(Bytecode::kJump, 0);
-  optimizer()->WriteJump(&jump, &target);
-  CHECK_EQ(write_count(), 1);
-  CHECK_EQ(jump, last_written());
-
-  BytecodeNode add(Bytecode::kAdd, Register(0).ToOperand(), 1);
-  optimizer()->Write(&add);
-  CHECK_EQ(write_count(), 1);
-  CHECK_EQ(jump, last_written());
-}
-
-TEST_F(BytecodeDeadCodeOptimizerTest, DeadCodeStillDeadAfterConditinalJump) {
-  BytecodeNode ret(Bytecode::kReturn);
-  optimizer()->Write(&ret);
-  CHECK_EQ(write_count(), 1);
-  CHECK_EQ(ret, last_written());
-
-  BytecodeLabel target;
-  BytecodeNode jump(Bytecode::kJumpIfTrue, 0);
-  optimizer()->WriteJump(&jump, &target);
-  CHECK_EQ(write_count(), 1);
-  CHECK_EQ(ret, last_written());
-
-  BytecodeNode add(Bytecode::kAdd, Register(0).ToOperand(), 1);
-  optimizer()->Write(&add);
-  CHECK_EQ(write_count(), 1);
-  CHECK_EQ(ret, last_written());
-}
-
-TEST_F(BytecodeDeadCodeOptimizerTest, CodeLiveAfterLabelBind) {
-  BytecodeNode ret(Bytecode::kReturn);
-  optimizer()->Write(&ret);
-  CHECK_EQ(write_count(), 1);
-  CHECK_EQ(ret, last_written());
-
-  BytecodeLabel target;
-  optimizer()->BindLabel(&target);
-
-  BytecodeNode add(Bytecode::kAdd, Register(0).ToOperand(), 1);
-  optimizer()->Write(&add);
-  CHECK_EQ(write_count(), 2);
-  CHECK_EQ(add, last_written());
-}
-
-}  // namespace interpreter
-}  // namespace internal
-}  // namespace v8

test/unittests/unittests.gyp

@@ -103,7 +103,6 @@
       'interpreter/bytecode-array-iterator-unittest.cc',
       'interpreter/bytecode-array-random-iterator-unittest.cc',
       'interpreter/bytecode-array-writer-unittest.cc',
-      'interpreter/bytecode-dead-code-optimizer-unittest.cc',
       'interpreter/bytecode-decoder-unittest.cc',
       'interpreter/bytecode-operands-unittest.cc',
       'interpreter/bytecode-pipeline-unittest.cc',