Revert of [interpeter] Move to table based peephole optimizer. (patchset #38...

Revert of [interpeter] Move to table based peephole optimizer. (patchset #38 id:730001 of https://codereview.chromium.org/2118183002/ )

Reason for revert:
Break MIPS port.

Original issue's description:
> [interpeter] Move to table based peephole optimizer.
>
> Introduces a lookup table for peephole optimizations.
>
> Fixes some tests using BytecodePeepholeOptimizer::Write() that should
> have been update to use BytecodePeepholeOptimizer::WriteJump().
>
> BUG=v8:4280
> LOG=N
>
> Committed: https://crrev.com/f4234422b93b21a286b0f31799009bcbe8b90b9e
> Cr-Commit-Position: refs/heads/master@{#37819}

TBR=rmcilroy@chromium.org,machenbach@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4280

Review-Url: https://codereview.chromium.org/2161563002
Cr-Commit-Position: refs/heads/master@{#37821}

BUILD.gn

@@ -86,7 +86,6 @@ if (v8_enable_gdbjit == "") {
   }
 }
 
-v8_generated_peephole_source = "$target_gen_dir/bytecode-peephole-table.cc"
 v8_random_seed = "314159265"
 v8_toolset_for_shell = "host"
 
@@ -650,25 +649,6 @@ action("run_mksnapshot") {
   }
 }
 
-action("run_mkpeephole") {
-  visibility = [ ":*" ]  # Only targets in this file can depend on this.
-
-  deps = [ ":mkpeephole($v8_snapshot_toolchain)" ]
-
-  outputs = [ v8_generated_peephole_source ]
-
-  sources = []
-
-  script = "tools/run.py"
-
-  args = [
-    "./" + rebase_path(get_label_info(":mkpeephole($v8_snapshot_toolchain)",
-                                      "root_out_dir") + "/mkpeephole",
-                       root_build_dir),
-    rebase_path(v8_generated_peephole_source, root_build_dir),
-  ]
-}
-
 action("v8_dump_build_config") {
   script = "tools/testrunner/utils/dump_build_config.py"
   outputs = [
@@ -1331,7 +1311,6 @@ v8_source_set("v8_base") {
     "src/interpreter/bytecode-label.h",
     "src/interpreter/bytecode-peephole-optimizer.cc",
     "src/interpreter/bytecode-peephole-optimizer.h",
-    "src/interpreter/bytecode-peephole-table.h",
     "src/interpreter/bytecode-pipeline.cc",
     "src/interpreter/bytecode-pipeline.h",
     "src/interpreter/bytecode-register-allocator.cc",
@@ -1930,9 +1909,6 @@ v8_source_set("v8_base") {
     ":v8_libsampler",
   ]
 
-  sources += [ v8_generated_peephole_source ]
-  deps += [ ":run_mkpeephole" ]
-
   if (is_win) {
     # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
     cflags = [ "/wd4267" ]
@@ -2156,31 +2132,6 @@ if (current_toolchain == v8_snapshot_toolchain) {
   }
 }
 
-v8_executable("mkpeephole") {
-  # mkpeephole needs to be built for the build host so the peephole lookup
-  # table can built during build. The table depends on the properties of
-  # bytecodes that are described in bytecodes.{cc,h}.
-  visibility = [ ":*" ]  # Only targets in this file can depend on this.
-
-  sources = [
-    "src/interpreter/bytecode-peephole-optimizer.h",
-    "src/interpreter/bytecodes.cc",
-    "src/interpreter/bytecodes.h",
-    "src/interpreter/mkpeephole.cc",
-  ]
-
-  configs = [
-    ":external_config",
-    ":internal_config",
-  ]
-
-  deps = [
-    ":v8_libbase",
-    "//build/config/sanitizers:deps",
-    "//build/win:default_exe_manifest",
-  ]
-}
-
 ###############################################################################
 # Public targets
 #

src/interpreter/bytecode-peephole-optimizer.h

-// Copyright 2016 the V8 project authors. All rights reserved.
+// Copyright 2015 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_PEEPHOLE_OPTIMIZER_H_
 #define V8_INTERPRETER_BYTECODE_PEEPHOLE_OPTIMIZER_H_
 
-#include "src/interpreter/bytecode-peephole-table.h"
 #include "src/interpreter/bytecode-pipeline.h"
 
 namespace v8 {
@@ -13,7 +12,6 @@ namespace internal {
 namespace interpreter {
 
 class ConstantArrayBuilder;
-class BytecodePeepholeActionAndData;
 
 // An optimization stage for performing peephole optimizations on
 // generated bytecode. The optimizer may buffer one bytecode
@@ -34,26 +32,31 @@ class BytecodePeepholeOptimizer final : public BytecodePipelineStage,
       Handle<FixedArray> handler_table) override;
 
  private:
-#define DECLARE_ACTION(Action)          \
-  void Action(BytecodeNode* const node, \
-              const PeepholeActionAndData* const action_data = nullptr);
-  PEEPHOLE_ACTION_LIST(DECLARE_ACTION)
-#undef DECLARE_ACTION
-
-  void ApplyPeepholeAction(BytecodeNode* const node);
+  BytecodeNode* OptimizeAndEmitLast(BytecodeNode* current);
+  BytecodeNode* Optimize(BytecodeNode* current);
   void Flush();
+
+  void TryToRemoveLastExpressionPosition(const BytecodeNode* const current);
+  bool TransformCurrentBytecode(BytecodeNode* const current);
+  bool TransformLastAndCurrentBytecodes(BytecodeNode* const current);
+  bool CanElideCurrent(const BytecodeNode* const current) const;
+  bool CanElideLast(const BytecodeNode* const current) const;
   bool CanElideLastBasedOnSourcePosition(
       const BytecodeNode* const current) const;
+
+  // Simple substitution methods.
+  bool RemoveToBooleanFromJump(BytecodeNode* const current);
+  bool RemoveToBooleanFromLogicalNot(BytecodeNode* const current);
+
   void InvalidateLast();
   bool LastIsValid() const;
   void SetLast(const BytecodeNode* const node);
 
+  bool LastBytecodePutsNameInAccumulator() const;
+
   Handle<Object> GetConstantForIndexOperand(const BytecodeNode* const node,
                                             int index) const;
 
-  BytecodePipelineStage* next_stage() const { return next_stage_; }
-  BytecodeNode* last() { return &last_; }
-
   ConstantArrayBuilder* constant_array_builder_;
   BytecodePipelineStage* next_stage_;
   BytecodeNode last_;

src/interpreter/bytecode-peephole-table.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_PEEPHOLE_TABLE_H_
-#define V8_INTERPRETER_BYTECODE_PEEPHOLE_TABLE_H_
-
-#include "src/interpreter/bytecodes.h"
-
-namespace v8 {
-namespace internal {
-namespace interpreter {
-
-#define PEEPHOLE_NON_JUMP_ACTION_LIST(V)            \
-  V(DefaultAction)                                  \
-  V(UpdateLastAction)                               \
-  V(UpdateLastIfSourceInfoPresentAction)            \
-  V(ElideCurrentAction)                             \
-  V(ElideCurrentIfOperand0MatchesAction)            \
-  V(ElideCurrentIfLoadingNameConstantAction)        \
-  V(ElideLastAction)                                \
-  V(ChangeBytecodeAction)                           \
-  V(TransformLdaStarToLdrLdarAction)                \
-  V(TransformLdaSmiBinaryOpToBinaryOpWithSmiAction) \
-  V(TransformLdaZeroBinaryOpToBinaryOpWithZeroAction)
-
-#define PEEPHOLE_JUMP_ACTION_LIST(V) \
-  V(DefaultJumpAction)               \
-  V(UpdateLastJumpAction)            \
-  V(ChangeJumpBytecodeAction)        \
-  V(ElideLastBeforeJumpAction)
-
-#define PEEPHOLE_ACTION_LIST(V)    \
-  PEEPHOLE_NON_JUMP_ACTION_LIST(V) \
-  PEEPHOLE_JUMP_ACTION_LIST(V)
-
-// Actions to take when a pair of bytes is encountered. A handler
-// exists for each action.
-enum class PeepholeAction : uint8_t {
-#define DECLARE_PEEPHOLE_ACTION(Action) k##Action,
-  PEEPHOLE_ACTION_LIST(DECLARE_PEEPHOLE_ACTION)
-#undef DECLARE_PEEPHOLE_ACTION
-};
-
-// Tuple of action to take when pair of bytecodes is encountered and
-// optional data to invoke handler with.
-struct PeepholeActionAndData final {
-  // Action to take when tuple of bytecodes encountered.
-  PeepholeAction action;
-
-  // Replacement bytecode (if valid).
-  Bytecode bytecode;
-};
-
-// Lookup table for matching pairs of bytecodes to peephole optimization
-// actions. The contents of the table are generated by mkpeephole.cc.
-struct PeepholeActionTable final {
- public:
-  static const PeepholeActionAndData* Lookup(Bytecode last, Bytecode current);
-
- private:
-  static const size_t kNumberOfBytecodes =
-      static_cast<size_t>(Bytecode::kLast) + 1;
-
-  static const PeepholeActionAndData row_data_[][kNumberOfBytecodes];
-  static const PeepholeActionAndData* const row_[kNumberOfBytecodes];
-};
-
-}  // namespace interpreter
-}  // namespace internal
-}  // namespace v8
-
-#endif  // V8_INTERPRETER_BYTECODE_PEEPHOLE_TABLE_H_

src/interpreter/bytecodes.cc

@@ -476,6 +476,11 @@ bool Bytecodes::IsLdarOrStar(Bytecode bytecode) {
   return bytecode == Bytecode::kLdar || bytecode == Bytecode::kStar;
 }
 
+// static
+bool Bytecodes::IsLdaSmiOrLdaZero(Bytecode bytecode) {
+  return bytecode == Bytecode::kLdaSmi || bytecode == Bytecode::kLdaZero;
+}
+
 // static
 bool Bytecodes::IsBytecodeWithScalableOperands(Bytecode bytecode) {
   switch (bytecode) {
@@ -503,11 +508,6 @@ bool Bytecodes::IsPrefixScalingBytecode(Bytecode bytecode) {
   }
 }
 
-// static
-bool Bytecodes::PutsNameInAccumulator(Bytecode bytecode) {
-  return bytecode == Bytecode::kToName || bytecode == Bytecode::kTypeOf;
-}
-
 // static
 bool Bytecodes::IsJumpOrReturn(Bytecode bytecode) {
   return bytecode == Bytecode::kReturn || IsJump(bytecode);

src/interpreter/bytecodes.h

@@ -514,15 +514,15 @@ class Bytecodes final {
   // Returns true if the bytecode is Ldar or Star.
   static bool IsLdarOrStar(Bytecode bytecode);
 
+  // Returns true if the bytecode is LdaSmi or LdaZero.
+  static bool IsLdaSmiOrLdaZero(Bytecode bytecode);
+
   // Returns true if the bytecode has wider operand forms.
   static bool IsBytecodeWithScalableOperands(Bytecode bytecode);
 
   // Returns true if the bytecode is a scaling prefix bytecode.
   static bool IsPrefixScalingBytecode(Bytecode bytecode);
 
-  // Returns true if |bytecode| puts a name in the accumulator.
-  static bool PutsNameInAccumulator(Bytecode bytecode);
-
   // Returns true if |operand_type| is any type of register operand.
   static bool IsRegisterOperandType(OperandType operand_type);
 

src/v8.gyp

@@ -35,7 +35,6 @@
     'v8_extra_library_files%': [],
     'v8_experimental_extra_library_files%': [],
     'mksnapshot_exec': '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)mksnapshot<(EXECUTABLE_SUFFIX)',
-    'mkpeephole_exec': '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)mkpeephole<(EXECUTABLE_SUFFIX)',
   },
   'includes': ['../gypfiles/toolchain.gypi', '../gypfiles/features.gypi'],
   'targets': [
@@ -383,31 +382,13 @@
         'v8_libbase',
         'v8_libsampler',
       ],
-      'objs': ['foo.o'],
       'variables': {
         'optimize': 'max',
       },
       'include_dirs+': [
         '..',
         '<(DEPTH)',
-        '<(SHARED_INTERMEDIATE_DIR)'
       ],
-      'actions':[{
-        'action_name': 'run mkpeephole',
-        'inputs': ['<(mkpeephole_exec)'],
-        'outputs': ['<(INTERMEDIATE_DIR)/bytecode-peephole-table.cc'],
-        'action': ['<(mkpeephole_exec)', '<(INTERMEDIATE_DIR)/bytecode-peephole-table.cc' ],
-        'process_outputs_as_sources': 1,
-        'conditions': [
-          ['want_separate_host_toolset==1', {
-            'dependencies': ['mkpeephole#host'],
-            'toolsets': ['host'],
-          }, {
-            'dependencies': ['mkpeephole'],
-            'toolsets': ['target'],
-          }],
-        ],
-      }],
       'sources': [  ### gcmole(all) ###
         '../include/v8-debug.h',
         '../include/v8-experimental.h',
@@ -948,7 +929,6 @@
         'interpreter/bytecode-label.h',
         'interpreter/bytecode-peephole-optimizer.cc',
         'interpreter/bytecode-peephole-optimizer.h',
-        'interpreter/bytecode-peephole-table.h',
         'interpreter/bytecode-pipeline.cc',
         'interpreter/bytecode-pipeline.h',
         'interpreter/bytecode-register.cc',
@@ -2335,26 +2315,5 @@
         }],
       ],
     },
-    {
-      'target_name': 'mkpeephole',
-      'type': 'executable',
-      'dependencies': [ 'v8_libbase' ],
-      'include_dirs+': [
-        '..',
-       ],
-      'sources': [
-        'interpreter/bytecode-peephole-table.h',
-        'interpreter/bytecodes.h',
-        'interpreter/bytecodes.cc',
-        'interpreter/mkpeephole.cc'
-      ],
-      'conditions': [
-        ['want_separate_host_toolset==1', {
-          'toolsets': ['host'],
-        }, {
-          'toolsets': ['target'],
-        }],
-      ],
-    },
   ],
 }

test/cctest/interpreter/bytecode_expectations/ConstVariable.golden

@@ -59,7 +59,7 @@ snippet: "
 "
 frame size: 3
 parameter count: 1
-bytecode array length: 31
+bytecode array length: 33
 bytecodes: [
                 B(LdaTheHole),
                 B(Star), R(0),
@@ -73,6 +73,7 @@ bytecodes: [
   /*   48 E> */ B(CallRuntime), U16(Runtime::kThrowReferenceError), R(2), U8(1),
                 B(CallRuntime), U16(Runtime::kThrowConstAssignError), R(0), U8(0),
                 B(Mov), R(1), R(0),
+                B(Ldar), R(0),
                 B(LdaUndefined),
   /*   55 S> */ B(Return),
 ]

test/cctest/interpreter/bytecode_expectations/LetVariable.golden

@@ -59,7 +59,7 @@ snippet: "
 "
 frame size: 3
 parameter count: 1
-bytecode array length: 26
+bytecode array length: 28
 bytecodes: [
                 B(LdaTheHole),
                 B(Star), R(0),
@@ -72,6 +72,7 @@ bytecodes: [
                 B(Star), R(2),
   /*   45 E> */ B(CallRuntime), U16(Runtime::kThrowReferenceError), R(2), U8(1),
                 B(Mov), R(1), R(0),
+                B(Ldar), R(0),
                 B(LdaUndefined),
   /*   52 S> */ B(Return),
 ]

test/unittests/interpreter/bytecode-peephole-optimizer-unittest.cc

@@ -134,10 +134,12 @@ TEST_F(BytecodePeepholeOptimizerTest, KeepStatementNop) {
 TEST_F(BytecodePeepholeOptimizerTest, KeepJumpIfToBooleanTrue) {
   BytecodeNode first(Bytecode::kLdaNull);
   BytecodeNode second(Bytecode::kJumpIfToBooleanTrue, 3);
-  BytecodeLabel label;
   optimizer()->Write(&first);
   CHECK_EQ(write_count(), 0);
-  optimizer()->WriteJump(&second, &label);
+  optimizer()->Write(&second);
+  CHECK_EQ(write_count(), 1);
+  CHECK_EQ(last_written(), first);
+  Flush();
   CHECK_EQ(write_count(), 2);
   CHECK_EQ(last_written(), second);
 }
@@ -145,12 +147,15 @@ TEST_F(BytecodePeepholeOptimizerTest, KeepJumpIfToBooleanTrue) {
 TEST_F(BytecodePeepholeOptimizerTest, ElideJumpIfToBooleanTrue) {
   BytecodeNode first(Bytecode::kLdaTrue);
   BytecodeNode second(Bytecode::kJumpIfToBooleanTrue, 3);
-  BytecodeLabel label;
   optimizer()->Write(&first);
   CHECK_EQ(write_count(), 0);
-  optimizer()->WriteJump(&second, &label);
+  optimizer()->Write(&second);
+  CHECK_EQ(write_count(), 1);
+  CHECK_EQ(last_written(), first);
+  Flush();
   CHECK_EQ(write_count(), 2);
-  CHECK_EQ(last_written(), second);
+  CHECK_EQ(last_written().bytecode(), Bytecode::kJumpIfTrue);
+  CHECK_EQ(last_written().operand(0), second.operand(0));
 }
 
 TEST_F(BytecodePeepholeOptimizerTest, KeepToBooleanLogicalNot) {
@@ -199,11 +204,12 @@ TEST_F(BytecodePeepholeOptimizerTest, StarRxLdarRx) {
   BytecodeNode first(Bytecode::kStar, Register(0).ToOperand());
   BytecodeNode second(Bytecode::kLdar, Register(0).ToOperand());
   optimizer()->Write(&first);
-  optimizer()->Write(&second);
   CHECK_EQ(write_count(), 0);
-  Flush();
+  optimizer()->Write(&second);
   CHECK_EQ(write_count(), 1);
   CHECK_EQ(last_written(), first);
+  Flush();
+  CHECK_EQ(write_count(), 1);
 }
 
 TEST_F(BytecodePeepholeOptimizerTest, StarRxLdarRxStatement) {
@@ -256,10 +262,11 @@ TEST_F(BytecodePeepholeOptimizerTest, ToNameToName) {
   BytecodeNode first(Bytecode::kToName);
   BytecodeNode second(Bytecode::kToName);
   optimizer()->Write(&first);
-  optimizer()->Write(&second);
   CHECK_EQ(write_count(), 0);
-  Flush();
+  optimizer()->Write(&second);
+  CHECK_EQ(write_count(), 1);
   CHECK_EQ(last_written(), first);
+  Flush();
   CHECK_EQ(write_count(), 1);
 }
 
@@ -267,11 +274,12 @@ TEST_F(BytecodePeepholeOptimizerTest, TypeOfToName) {
   BytecodeNode first(Bytecode::kTypeOf);
   BytecodeNode second(Bytecode::kToName);
   optimizer()->Write(&first);
-  optimizer()->Write(&second);
   CHECK_EQ(write_count(), 0);
-  Flush();
+  optimizer()->Write(&second);
   CHECK_EQ(write_count(), 1);
   CHECK_EQ(last_written(), first);
+  Flush();
+  CHECK_EQ(write_count(), 1);
 }
 
 TEST_F(BytecodePeepholeOptimizerTest, LdaConstantStringToName) {
@@ -281,11 +289,12 @@ TEST_F(BytecodePeepholeOptimizerTest, LdaConstantStringToName) {
   BytecodeNode first(Bytecode::kLdaConstant, static_cast<uint32_t>(index));
   BytecodeNode second(Bytecode::kToName);
   optimizer()->Write(&first);
-  optimizer()->Write(&second);
   CHECK_EQ(write_count(), 0);
-  Flush();
+  optimizer()->Write(&second);
   CHECK_EQ(write_count(), 1);
   CHECK_EQ(last_written(), first);
+  Flush();
+  CHECK_EQ(write_count(), 1);
 }
 
 TEST_F(BytecodePeepholeOptimizerTest, LdaConstantNumberToName) {