[interpreter] Remove mechanism for bytecode handler reuse

This was originally introduced to reuse large handlers, but now only
LdaContextSlot and LdaCurrentContextSlot remain (both roughly 2-300
bytes in size).

Since handler reuse complicates lazy (de)serialization and currently
doesn't seem to give us significant advantages, let's remove this.

Bug: v8:6624
Change-Id: I6f19952632e10bd67677a825bbcb46d580a9d5c8
Reviewed-on: https://chromium-review.googlesource.com/758642Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49254}

src/interpreter/bytecodes.h

@@ -840,46 +840,6 @@ class V8_EXPORT_PRIVATE Bytecodes final : public AllStatic {
     UNREACHABLE();
   }
 
-  // Returns true, iff the given bytecode reuses an existing handler. If so,
-  // the bytecode of the reused handler is written into {reused}.
-  static bool ReusesExistingHandler(Bytecode bytecode, Bytecode* reused) {
-    switch (bytecode) {
-      case Bytecode::kLdaImmutableContextSlot:
-        STATIC_ASSERT(static_cast<int>(Bytecode::kLdaContextSlot) <
-                      static_cast<int>(Bytecode::kLdaImmutableContextSlot));
-        *reused = Bytecode::kLdaContextSlot;
-        return true;
-      case Bytecode::kLdaImmutableCurrentContextSlot:
-        STATIC_ASSERT(
-            static_cast<int>(Bytecode::kLdaCurrentContextSlot) <
-            static_cast<int>(Bytecode::kLdaImmutableCurrentContextSlot));
-        *reused = Bytecode::kLdaCurrentContextSlot;
-        return true;
-      default:
-        return false;
-    }
-  }
-
-  static std::vector<Bytecode> AllBytecodesUsingHandler(Bytecode bytecode) {
-    Bytecode dummy;
-    USE(dummy);
-    switch (bytecode) {
-      case Bytecode::kLdaContextSlot:
-        DCHECK(
-            ReusesExistingHandler(Bytecode::kLdaImmutableContextSlot, &dummy));
-        DCHECK_EQ(bytecode, dummy);
-        return {bytecode, Bytecode::kLdaImmutableContextSlot};
-      case Bytecode::kLdaCurrentContextSlot:
-        DCHECK(ReusesExistingHandler(Bytecode::kLdaImmutableCurrentContextSlot,
-                                     &dummy));
-        DCHECK_EQ(bytecode, dummy);
-        return {bytecode, Bytecode::kLdaImmutableCurrentContextSlot};
-      default:
-        DCHECK(!ReusesExistingHandler(bytecode, &dummy));
-        return {bytecode};
-    }
-  }
-
   // Returns the size of |operand_type| for |operand_scale|.
   static OperandSize SizeOfOperand(OperandType operand_type,
                                    OperandScale operand_scale) {

src/interpreter/interpreter-generator.cc

@@ -311,8 +311,14 @@ IGNITION_HANDLER(LdaContextSlot, InterpreterAssembler) {
 // Load the object in |slot_index| of the context at |depth| in the context
 // chain starting at |context| into the accumulator.
 IGNITION_HANDLER(LdaImmutableContextSlot, InterpreterAssembler) {
-  // Same as LdaContextSlot, should never be called.
-  UNREACHABLE();
+  Node* reg_index = BytecodeOperandReg(0);
+  Node* context = LoadRegister(reg_index);
+  Node* slot_index = BytecodeOperandIdx(1);
+  Node* depth = BytecodeOperandUImm(2);
+  Node* slot_context = GetContextAtDepth(context, depth);
+  Node* result = LoadContextElement(slot_context, slot_index);
+  SetAccumulator(result);
+  Dispatch();
 }
 
 // LdaCurrentContextSlot <slot_index>
@@ -330,8 +336,11 @@ IGNITION_HANDLER(LdaCurrentContextSlot, InterpreterAssembler) {
 //
 // Load the object in |slot_index| of the current context into the accumulator.
 IGNITION_HANDLER(LdaImmutableCurrentContextSlot, InterpreterAssembler) {
-  // Same as LdaCurrentContextSlot, should never be called.
-  UNREACHABLE();
+  Node* slot_index = BytecodeOperandIdx(0);
+  Node* slot_context = GetContext();
+  Node* result = LoadContextElement(slot_context, slot_index);
+  SetAccumulator(result);
+  Dispatch();
 }
 
 // StaContextSlot <context> <slot_index> <depth>

src/interpreter/interpreter.cc

@@ -101,27 +101,14 @@ Code* Interpreter::GetAndMaybeDeserializeBytecodeHandler(
            Bytecodes::ToString(bytecode, operand_scale).c_str());
   }
 
-  // Some handlers are reused for several bytecodes. If we encounter such a
-  // bytecode, find the canonical handler, deserialize it, and write it into all
-  // slots in the dispatch table that (re)use it.
-
-  Bytecode maybe_reused_bytecode;
-  const bool reuses_existing_handler =
-      Bytecodes::ReusesExistingHandler(bytecode, &maybe_reused_bytecode);
-
-  Bytecode handler_bytecode =
-      reuses_existing_handler ? maybe_reused_bytecode : bytecode;
-
-  DCHECK(Bytecodes::BytecodeHasHandler(handler_bytecode, operand_scale));
-  code =
-      Snapshot::DeserializeHandler(isolate_, handler_bytecode, operand_scale);
+  DCHECK(Bytecodes::BytecodeHasHandler(bytecode, operand_scale));
+  code = Snapshot::DeserializeHandler(isolate_, bytecode, operand_scale);
 
   DCHECK(code->IsCode());
   DCHECK_EQ(code->kind(), Code::BYTECODE_HANDLER);
+  DCHECK(!isolate_->heap()->IsDeserializeLazyHandler(code));
 
-  for (Bytecode b : Bytecodes::AllBytecodesUsingHandler(handler_bytecode)) {
-    SetBytecodeHandler(b, operand_scale, code);
-  }
+  SetBytecodeHandler(bytecode, operand_scale, code);
 
   return code;
 }

src/interpreter/setup-interpreter-internal.cc

@@ -72,29 +72,12 @@ void SetupInterpreter::InstallBytecodeHandlers(Interpreter* interpreter) {
   DCHECK(interpreter->IsDispatchTableInitialized());
 }
 
-// static
-bool SetupInterpreter::ReuseExistingHandler(Address* dispatch_table,
-                                            Bytecode bytecode,
-                                            OperandScale operand_scale) {
-  Bytecode reused_bytecode;
-  if (!Bytecodes::ReusesExistingHandler(bytecode, &reused_bytecode)) {
-    return false;
-  }
-
-  size_t index = Interpreter::GetDispatchTableIndex(bytecode, operand_scale);
-  dispatch_table[index] = dispatch_table[Interpreter::GetDispatchTableIndex(
-      reused_bytecode, operand_scale)];
-
-  return true;
-}
-
 // static
 void SetupInterpreter::InstallBytecodeHandler(Isolate* isolate,
                                               Address* dispatch_table,
                                               Bytecode bytecode,
                                               OperandScale operand_scale) {
   if (!Bytecodes::BytecodeHasHandler(bytecode, operand_scale)) return;
-  if (ReuseExistingHandler(dispatch_table, bytecode, operand_scale)) return;
 
   size_t index = Interpreter::GetDispatchTableIndex(bytecode, operand_scale);
   Handle<Code> code = GenerateBytecodeHandler(isolate, bytecode, operand_scale);

src/interpreter/setup-interpreter.h

@@ -19,10 +19,6 @@ class SetupInterpreter {
   static void InstallBytecodeHandlers(Interpreter* interpreter);
 
  private:
-  // In the case of bytecodes that share handler implementations, copy the code
-  // into the bytecode's dispatcher table entry and return true.
-  static bool ReuseExistingHandler(Address* dispatch_table, Bytecode bytecode,
-                                   OperandScale operand_scale);
   // Generates handler for given |bytecode| and |operand_scale|
   // and installs it into the |dispatch_table|.
   static void InstallBytecodeHandler(Isolate* isolate, Address* dispatch_table,

src/snapshot/builtin-deserializer-allocator.cc

@@ -95,7 +95,7 @@ BuiltinDeserializerAllocator::CreateReservationsForEagerBuiltinsAndHandlers() {
 
   BSU::ForEachBytecode(
       [=, &result](Bytecode bytecode, OperandScale operand_scale) {
-        if (!BSU::BytecodeHasDedicatedHandler(bytecode, operand_scale)) {
+        if (!Bytecodes::BytecodeHasHandler(bytecode, operand_scale)) {
           // Bytecodes without a handler don't require a reservation.
           return;
         } else if (FLAG_lazy_handler_deserialization &&
@@ -188,7 +188,7 @@ void BuiltinDeserializerAllocator::InitializeFromReservations(
 
   BSU::ForEachBytecode(
       [=, &reservation_index](Bytecode bytecode, OperandScale operand_scale) {
-        if (!BSU::BytecodeHasDedicatedHandler(bytecode, operand_scale)) {
+        if (!Bytecodes::BytecodeHasHandler(bytecode, operand_scale)) {
           // Bytecodes without a handler don't have a reservation.
           return;
         } else if (FLAG_lazy_handler_deserialization &&

src/snapshot/builtin-deserializer.cc

@@ -80,7 +80,7 @@ void BuiltinDeserializer::DeserializeEagerBuiltinsAndHandlers() {
 
   BSU::ForEachBytecode([=](Bytecode bytecode, OperandScale operand_scale) {
     // Bytecodes without a dedicated handler are patched up in a second pass.
-    if (!BSU::BytecodeHasDedicatedHandler(bytecode, operand_scale)) return;
+    if (!Bytecodes::BytecodeHasHandler(bytecode, operand_scale)) return;
 
     // If lazy-deserialization is enabled and the current bytecode is lazy,
     // we write the generic LazyDeserialization handler into the dispatch table
@@ -95,24 +95,11 @@ void BuiltinDeserializer::DeserializeEagerBuiltinsAndHandlers() {
 
   // Patch up holes in the dispatch table.
 
-  DCHECK(BSU::BytecodeHasDedicatedHandler(Bytecode::kIllegal,
-                                          OperandScale::kSingle));
   Code* illegal_handler = interpreter->GetBytecodeHandler(
       Bytecode::kIllegal, OperandScale::kSingle);
 
   BSU::ForEachBytecode([=](Bytecode bytecode, OperandScale operand_scale) {
-    if (BSU::BytecodeHasDedicatedHandler(bytecode, operand_scale)) return;
-
-    Bytecode maybe_reused_bytecode;
-    if (Bytecodes::ReusesExistingHandler(bytecode, &maybe_reused_bytecode)) {
-      interpreter->SetBytecodeHandler(
-          bytecode, operand_scale,
-          interpreter->GetBytecodeHandler(maybe_reused_bytecode,
-                                          operand_scale));
-      return;
-    }
-
-    DCHECK(!Bytecodes::BytecodeHasHandler(bytecode, operand_scale));
+    if (Bytecodes::BytecodeHasHandler(bytecode, operand_scale)) return;
     interpreter->SetBytecodeHandler(bytecode, operand_scale, illegal_handler);
   });
 
@@ -158,7 +145,7 @@ Code* BuiltinDeserializer::DeserializeBuiltinRaw(int builtin_id) {
 Code* BuiltinDeserializer::DeserializeHandlerRaw(Bytecode bytecode,
                                                  OperandScale operand_scale) {
   DCHECK(!AllowHeapAllocation::IsAllowed());
-  DCHECK(BSU::BytecodeHasDedicatedHandler(bytecode, operand_scale));
+  DCHECK(Bytecodes::BytecodeHasHandler(bytecode, operand_scale));
 
   const int code_object_id = BSU::BytecodeToIndex(bytecode, operand_scale);
   DeserializingCodeObjectScope scope(this, code_object_id);

src/snapshot/builtin-serializer.cc

@@ -39,7 +39,7 @@ void BuiltinSerializer::SerializeBuiltinsAndHandlers() {
 
   BSU::ForEachBytecode([=](Bytecode bytecode, OperandScale operand_scale) {
     SetHandlerOffset(bytecode, operand_scale, sink_.Position());
-    if (!BSU::BytecodeHasDedicatedHandler(bytecode, operand_scale)) return;
+    if (!Bytecodes::BytecodeHasHandler(bytecode, operand_scale)) return;
 
     SerializeHandler(
         isolate()->interpreter()->GetBytecodeHandler(bytecode, operand_scale));

src/snapshot/builtin-snapshot-utils.cc

@@ -63,19 +63,5 @@ void BuiltinSnapshotUtils::ForEachBytecode(
   }
 }
 
-// static
-bool BuiltinSnapshotUtils::BytecodeHasDedicatedHandler(
-    Bytecode bytecode, OperandScale operand_scale) {
-  // Some bytecodes don't have a handler. The dispatch table contains the
-  // kIllegal handler in these slots.
-  if (!Bytecodes::BytecodeHasHandler(bytecode, operand_scale)) return false;
-
-  // Some handlers are reused for several bytecodes.
-  Bytecode dummy;
-  if (Bytecodes::ReusesExistingHandler(bytecode, &dummy)) return false;
-
-  return true;
-}
-
 }  // namespace internal
 }  // namespace v8

src/snapshot/builtin-snapshot-utils.h

@@ -31,8 +31,8 @@ class BuiltinSnapshotUtils : public AllStatic {
 
   // The number of code objects in the builtin snapshot.
   // TODO(jgruber): This could be reduced by a bit since not every
-  // {bytecode, operand_scale} combination has an associated handler, and some
-  // handlers are reused (see BytecodeHasDedicatedHandler).
+  // {bytecode, operand_scale} combination has an associated handler
+  // (see Bytecodes::BytecodeHasHandler).
   static const int kNumberOfCodeObjects = kNumberOfBuiltins + kNumberOfHandlers;
 
   // Indexes into the offsets vector contained in snapshot.
@@ -48,11 +48,6 @@ class BuiltinSnapshotUtils : public AllStatic {
   // Iteration over all {bytecode,operand_scale} pairs. Implemented here since
   // (de)serialization depends on the iteration order.
   static void ForEachBytecode(std::function<void(Bytecode, OperandScale)> f);
-
-  // True, iff the given {bytecode,operand_scale} has a dedicated handler, where
-  // dedicated means: a handler exists, and it does not reuse another handler.
-  static bool BytecodeHasDedicatedHandler(Bytecode bytecode,
-                                          OperandScale operand_scale);
 };
 
 }  // namespace internal