[interpeter] Move to table based peephole optimizer.

src/interpreter/bytecode-peephole-optimizer.cc

Index: src/interpreter/bytecode-peephole-optimizer.cc
diff --git a/src/interpreter/bytecode-peephole-optimizer.cc b/src/interpreter/bytecode-peephole-optimizer.cc
index cd668d4a67fa1d551a81605e7941b929e17a77af..36c252a3d7c229cb9c02832d4b5f59b4f3811a06 100644
--- a/src/interpreter/bytecode-peephole-optimizer.cc
+++ b/src/interpreter/bytecode-peephole-optimizer.cc
@@ -1,4 +1,4 @@
-// Copyright 2015 the V8 project authors. All rights reserved.
+// 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.
 
@@ -29,21 +29,6 @@ Handle<BytecodeArray> BytecodePeepholeOptimizer::ToBytecodeArray(
 }
 
 // override
-void BytecodePeepholeOptimizer::Write(BytecodeNode* node) {
-  node = OptimizeAndEmitLast(node);
-  if (node != nullptr) {
-    SetLast(node);
-  }
-}
-
-// override
-void BytecodePeepholeOptimizer::WriteJump(BytecodeNode* node,
-                                          BytecodeLabel* label) {
-  node = OptimizeAndEmitLast(node);
-  next_stage_->WriteJump(node, label);
-}
-
-// override
 void BytecodePeepholeOptimizer::BindLabel(BytecodeLabel* label) {
   Flush();
   next_stage_->BindLabel(label);
@@ -52,14 +37,22 @@ void BytecodePeepholeOptimizer::BindLabel(BytecodeLabel* label) {
 // override
 void BytecodePeepholeOptimizer::BindLabel(const BytecodeLabel& target,
                                           BytecodeLabel* label) {
-  // There is no need to flush here, it will have been flushed when |target|
-  // was bound.
+  // There is no need to flush here, it will have been flushed when
+  // |target| was bound.
   next_stage_->BindLabel(target, label);
 }
 
+// override
+void BytecodePeepholeOptimizer::WriteJump(BytecodeNode* node,
+                                          BytecodeLabel* label) {
+  Optimize(node);
+  next_stage()->WriteJump(node, label);
+}
+
+// override
+void BytecodePeepholeOptimizer::Write(BytecodeNode* node) { Optimize(node); }

[rmcilroy, 2016/07/13 13:51:41]

Could we give this a new name, since it does the optimization and also writes
nodes to the next stage. Also, is there any way we could make WriteJump and
Write behave similarly with respect to sending things on to the next_stage
(i.e., WriteJump does it outside Optimize, but Write doesn't).

[oth, 2016/07/15 13:09:40]

Renamed ApplyPeepholeAction().

The answer to the rest is a little long.

Basically, per the comment in ApplyPeephole() having a single table gives the
best performance. Having different handler signatures and tables for applying
jump and non-jumps is slower.

For non-jump bytecodes the handlers run to completion based on the action.
Sometimes they emit the incoming node, sometimes not. Conditionally check this
after ApplyPeepholeAction slows performance.

WriteJump() always emits the incoming jump bytecode and has an additional label
argument. We don't really want this as part of the handler signature because
passing it again affects performance.

Happy to discuss, but we're in a local perf maxima. Added comments in Write()
and WriteJump().

[rmcilroy, 2016/07/15 15:31:48]

I see, right enough forgot about the label. Ok works for me, thanks for the
comment. 

+
 void BytecodePeepholeOptimizer::Flush() {
-  // TODO(oth/rmcilroy): We could check CanElideLast() here to potentially
-  // eliminate last rather than writing it.

[rmcilroy, 2016/07/13 13:51:40]

No longer relevent?

[oth, 2016/07/15 13:09:40]

CanElideLast() is gone.

There's no information about the surrounding code so the only safe elision here
would be when last_ is a NOP without source info. Added a new rule to avoid this
state and a check in SetLast that existing rules don't try to get there.

[rmcilroy, 2016/07/15 15:31:48]

Great, thanks!

   if (LastIsValid()) {
     next_stage_->Write(&last_);
     InvalidateLast();
@@ -86,51 +79,6 @@ Handle<Object> BytecodePeepholeOptimizer::GetConstantForIndexOperand(
   return constant_array_builder_->At(index_operand);
 }
 
-bool BytecodePeepholeOptimizer::LastBytecodePutsNameInAccumulator() const {
-  DCHECK(LastIsValid());
-  return (last_.bytecode() == Bytecode::kTypeOf ||
-          last_.bytecode() == Bytecode::kToName ||
-          (last_.bytecode() == Bytecode::kLdaConstant &&
-           GetConstantForIndexOperand(&last_, 0)->IsName()));
-}
-
-void BytecodePeepholeOptimizer::TryToRemoveLastExpressionPosition(
-    const BytecodeNode* const current) {
-  if (current->source_info().is_valid() &&
-      last_.source_info().is_expression() &&
-      Bytecodes::IsWithoutExternalSideEffects(last_.bytecode())) {
-    // The last bytecode has been marked as expression. It has no
-    // external effects so can't throw and the current bytecode is a
-    // source position. Remove the expression position on the last
-    // bytecode to open up potential peephole optimizations and to
-    // save the memory and perf cost of storing the unneeded
-    // expression position.
-    last_.source_info().set_invalid();
-  }
-}
-
-bool BytecodePeepholeOptimizer::CanElideCurrent(
-    const BytecodeNode* const current) const {
-  if (Bytecodes::IsLdarOrStar(last_.bytecode()) &&
-      Bytecodes::IsLdarOrStar(current->bytecode()) &&
-      current->operand(0) == last_.operand(0)) {
-    // Ldar and Star make the accumulator and register hold equivalent
-    // values. Only the first bytecode is needed if there's a sequence
-    // of back-to-back Ldar and Star bytecodes with the same operand.
-    return true;
-  } else if (current->bytecode() == Bytecode::kToName &&
-             LastBytecodePutsNameInAccumulator()) {
-    // If the previous bytecode ensured a name was in the accumulator,
-    // the type coercion ToName() can be elided.
-    return true;
-  } else {
-    // Additional candidates for eliding current:
-    // (i) current is Nop.
-    // (ii) ToNumber if the last puts a number in the accumulator.
-    return false;
-  }
-}
-
 bool BytecodePeepholeOptimizer::CanElideLastBasedOnSourcePosition(
     const BytecodeNode* const current) const {
   //
@@ -156,10 +104,7 @@ bool BytecodePeepholeOptimizer::CanElideLastBasedOnSourcePosition(
   // The last bytecode can be elided for the MAYBE cases if the last

[rmcilroy, 2016/07/13 13:51:41]

What do we do with the MAYBE cases now? AFAIKT we don't do anything special here
any longer (so they are NO now), but we try to defer the emittion of the
expression positions until they are needed for throws in the
BytecodeArrayBuilder. If so, could you update this comment?

[oth, 2016/07/15 13:09:40]

Done. Hopefully clearer now. We've not treated MAYBE differently from NO for a
good while. PTAL at the wording.

[rmcilroy, 2016/07/15 15:31:48]

SGTM, thanks.

   // bytecode is known not to throw. If it throws, the system would
   // not have correct stack trace information. The appropriate check
-  // for this would be Bytecodes::IsWithoutExternalSideEffects(),
-  // which is checked in
-  // BytecodePeepholeOptimizer::TransformLastAndCurrentBytecodes() to
-  // keep the check here simple.
+  // for this would be Bytecodes::IsWithoutExternalSideEffects().
   //
   // In rare cases, bytecode generation produces consecutive bytecodes
   // with the same expression positions. In principle, the latter of
@@ -189,13 +134,21 @@ void TransformLdaStarToLdrLdar(Bytecode new_bytecode, BytecodeNode* const last,
   current->set_bytecode(Bytecode::kLdar, current->operand(0));
 }
 
-void TransformToBinaryOpWithSmiOnRhs(Bytecode new_bytecode,
-                                     BytecodeNode* const last,
-                                     BytecodeNode* const current) {
-  DCHECK(Bytecodes::IsLdaSmiOrLdaZero(last->bytecode()));
-  uint32_t imm_operand =
-      last->bytecode() == Bytecode::kLdaSmi ? last->operand(0) : 0;
-  current->set_bytecode(new_bytecode, imm_operand, current->operand(0));
+void TransformLdaSmiBinaryOpToBinaryOpWithSmi(Bytecode new_bytecode,
+                                              BytecodeNode* const last,
+                                              BytecodeNode* const current) {
+  DCHECK_EQ(last->bytecode(), Bytecode::kLdaSmi);
+  current->set_bytecode(new_bytecode, last->operand(0), current->operand(0));
+  if (last->source_info().is_valid()) {
+    current->source_info().Clone(last->source_info());
+  }
+}
+
+void TransformLdaZeroBinaryOpToBinaryOpWithZero(Bytecode new_bytecode,
+                                                BytecodeNode* const last,
+                                                BytecodeNode* const current) {
+  DCHECK_EQ(last->bytecode(), Bytecode::kLdaZero);
+  current->set_bytecode(new_bytecode, 0, current->operand(0));
   if (last->source_info().is_valid()) {
     current->source_info().Clone(last->source_info());
   }
@@ -203,171 +156,191 @@ void TransformToBinaryOpWithSmiOnRhs(Bytecode new_bytecode,
 
 }  // namespace
 
-bool BytecodePeepholeOptimizer::TransformLastAndCurrentBytecodes(
-    BytecodeNode* const current) {
-  if (current->bytecode() == Bytecode::kStar &&
-      !current->source_info().is_statement()) {
-    // Note: If the Star is tagged with a statement position, we can't
-    // perform this transform as the store to the register will
-    // have the wrong ordering for stepping in the debugger.
-    switch (last_.bytecode()) {
-      case Bytecode::kLdaNamedProperty:
-        TransformLdaStarToLdrLdar(Bytecode::kLdrNamedProperty, &last_, current);
-        return true;
-      case Bytecode::kLdaKeyedProperty:
-        TransformLdaStarToLdrLdar(Bytecode::kLdrKeyedProperty, &last_, current);
-        return true;
-      case Bytecode::kLdaGlobal:
-        TransformLdaStarToLdrLdar(Bytecode::kLdrGlobal, &last_, current);
-        return true;
-      case Bytecode::kLdaContextSlot:
-        TransformLdaStarToLdrLdar(Bytecode::kLdrContextSlot, &last_, current);
-        return true;
-      case Bytecode::kLdaUndefined:
-        TransformLdaStarToLdrLdar(Bytecode::kLdrUndefined, &last_, current);
-        return true;
-      default:
-        break;
-    }
-  } else if (Bytecodes::IsLdaSmiOrLdaZero(last_.bytecode()) &&
-             (!last_.source_info().is_valid() ||
-              !current->source_info().is_valid())) {
-    switch (current->bytecode()) {
-      case Bytecode::kAdd:
-        TransformToBinaryOpWithSmiOnRhs(Bytecode::kAddSmi, &last_, current);
-        InvalidateLast();
-        return true;
-      case Bytecode::kSub:
-        TransformToBinaryOpWithSmiOnRhs(Bytecode::kSubSmi, &last_, current);
-        InvalidateLast();
-        return true;
-      case Bytecode::kBitwiseOr:
-        TransformToBinaryOpWithSmiOnRhs(Bytecode::kBitwiseOrSmi, &last_,
-                                        current);
-        InvalidateLast();
-        return true;
-      case Bytecode::kBitwiseAnd:
-        TransformToBinaryOpWithSmiOnRhs(Bytecode::kBitwiseAndSmi, &last_,
-                                        current);
-        InvalidateLast();
-        return true;
-      case Bytecode::kShiftLeft:
-        TransformToBinaryOpWithSmiOnRhs(Bytecode::kShiftLeftSmi, &last_,
-                                        current);
-        InvalidateLast();
-        return true;
-      case Bytecode::kShiftRight:
-        TransformToBinaryOpWithSmiOnRhs(Bytecode::kShiftRightSmi, &last_,
-                                        current);
-        InvalidateLast();
-        return true;
-      default:
-        break;
-    }
-  }
+void BytecodePeepholeOptimizer::DefaultAction(
+    BytecodeNode* const node, const PeepholeActionAndData* action_data) {
+  DCHECK(LastIsValid());
+  DCHECK(!Bytecodes::IsJump(node->bytecode()));
+
+  next_stage()->Write(last());
+  SetLast(node);
+}
+
+void BytecodePeepholeOptimizer::UpdateLastAction(
+    BytecodeNode* const node, const PeepholeActionAndData* action_data) {
+  DCHECK(!LastIsValid());
+  DCHECK(!Bytecodes::IsJump(node->bytecode()));
 
-  return false;
+  SetLast(node);
 }
 
-bool BytecodePeepholeOptimizer::RemoveToBooleanFromJump(
-    BytecodeNode* const current) {
-  bool can_remove = Bytecodes::IsJumpIfToBoolean(current->bytecode()) &&
-                    Bytecodes::WritesBooleanToAccumulator(last_.bytecode());
-  if (can_remove) {
-    // Conditional jumps with boolean conditions are emiitted in
-    // ToBoolean form by the bytecode array builder,
-    // i.e. JumpIfToBooleanTrue rather JumpIfTrue. The ToBoolean
-    // element can be removed if the previous bytecode put a boolean
-    // value in the accumulator.
-    Bytecode jump = Bytecodes::GetJumpWithoutToBoolean(current->bytecode());
-    current->set_bytecode(jump, current->operand(0));
+void BytecodePeepholeOptimizer::ElideCurrentAction(
+    BytecodeNode* const node, const PeepholeActionAndData* action_data) {
+  DCHECK(LastIsValid());
+  DCHECK(!Bytecodes::IsJump(node->bytecode()));
+
+  if (node->source_info().is_valid()) {
+    // Preserve the source information by replacing the node bytecode
+    // with a no op bytecode.
+    node->set_bytecode(Bytecode::kNop);
+    DefaultAction(node);
+  } else {
+    // Nothing to do, keep last and wait for next bytecode to pair with it.
   }
-  return can_remove;
 }
 
-bool BytecodePeepholeOptimizer::RemoveToBooleanFromLogicalNot(
-    BytecodeNode* const current) {
-  bool can_remove = current->bytecode() == Bytecode::kToBooleanLogicalNot &&
-                    Bytecodes::WritesBooleanToAccumulator(last_.bytecode());
-  if (can_remove) {
-    // Logical-nots are emitted in ToBoolean form by the bytecode array
-    // builder, The ToBoolean element can be removed if the previous bytecode
-    // put a boolean value in the accumulator.
-    current->set_bytecode(Bytecode::kLogicalNot);
+void BytecodePeepholeOptimizer::ElideCurrentIfOperand0MatchesAction(
+    BytecodeNode* const node, const PeepholeActionAndData* action_data) {
+  DCHECK(LastIsValid());
+  DCHECK(!Bytecodes::IsJump(node->bytecode()));
+
+  if (last()->operand(0) == node->operand(0)) {
+    ElideCurrentAction(node);
+  } else {
+    DefaultAction(node);
   }
-  return can_remove;
 }
 
-bool BytecodePeepholeOptimizer::TransformCurrentBytecode(
-    BytecodeNode* const current) {
-  return RemoveToBooleanFromJump(current) ||
-         RemoveToBooleanFromLogicalNot(current);
+void BytecodePeepholeOptimizer::ElideCurrentIfLoadingNameConstantAction(
+    BytecodeNode* const node, const PeepholeActionAndData* action_data) {
+  DCHECK_EQ(last()->bytecode(), Bytecode::kLdaConstant);
+  DCHECK(!Bytecodes::IsJump(node->bytecode()));
+
+  if (GetConstantForIndexOperand(last(), 0)->IsName()) {
+    ElideCurrentAction(node);
+  } else {
+    DefaultAction(node);
+  }
 }
 
-bool BytecodePeepholeOptimizer::CanElideLast(
-    const BytecodeNode* const current) const {
-  if (last_.bytecode() == Bytecode::kNop) {
-    // Nop are placeholders for holding source position information.
-    return true;
-  } else if (Bytecodes::IsAccumulatorLoadWithoutEffects(current->bytecode()) &&
-             Bytecodes::IsAccumulatorLoadWithoutEffects(last_.bytecode())) {
-    // The accumulator is invisible to the debugger. If there is a sequence of
-    // consecutive accumulator loads (that don't have side effects) then only
-    // the final load is potentially visible.
-    return true;
-  } else if (Bytecodes::GetAccumulatorUse(current->bytecode()) ==
-                 AccumulatorUse::kWrite &&
-             Bytecodes::IsAccumulatorLoadWithoutEffects(last_.bytecode())) {
-    // The current instruction clobbers the accumulator without reading it. The
-    // load in the last instruction can be elided as it has no effect.
-    return true;
+void BytecodePeepholeOptimizer::ElideLastAction(
+    BytecodeNode* const node, const PeepholeActionAndData* action_data) {
+  DCHECK(LastIsValid());
+  DCHECK(!Bytecodes::IsJump(node->bytecode()));
+
+  if (CanElideLastBasedOnSourcePosition(node)) {
+    if (last()->source_info().is_valid()) {
+      // Node can not be valid per CanElideLastBasedOnSourcePosition().

[rmcilroy, 2016/07/13 13:51:41]

Can we DCHECK this?

[oth, 2016/07/15 13:09:40]

Hey! The code has just called CanElideLastBasedOnSourcePosition() and the
comment is referring to this. Shouldn't need to DCHECK condition still holds
here.

[rmcilroy, 2016/07/15 15:31:48]

Right, sorry I was confused since the comment sounded like the node is not valid
at this point (not the source position). Could the comment be clearer that it is
the source position that isn't valid?

[oth, 2016/07/15 17:27:25]

Definitely. Updated the comment.

+      node->source_info().Clone(last()->source_info());
+    }
+    SetLast(node);
   } else {
-    return false;
+    DefaultAction(node);
   }
 }
 
-BytecodeNode* BytecodePeepholeOptimizer::Optimize(BytecodeNode* current) {
-  TryToRemoveLastExpressionPosition(current);
-  if (TransformCurrentBytecode(current) ||
-      TransformLastAndCurrentBytecodes(current)) {
-    return current;
+void BytecodePeepholeOptimizer::ChangeBytecodeAction(
+    BytecodeNode* const node, const PeepholeActionAndData* action_data) {
+  DCHECK(LastIsValid());
+  DCHECK(!Bytecodes::IsJump(node->bytecode()));
+
+  node->set_bytecode(action_data->bytecode);
+  DefaultAction(node);
+}
+
+void BytecodePeepholeOptimizer::TransformLdaStarToLdrLdarAction(
+    BytecodeNode* const node, const PeepholeActionAndData* action_data) {
+  DCHECK(LastIsValid());
+  DCHECK(!Bytecodes::IsJump(node->bytecode()));
+
+  if (!node->source_info().is_statement()) {
+    TransformLdaStarToLdrLdar(action_data->bytecode, last(), node);
   }
+  DefaultAction(node);
+}
 
-  if (CanElideCurrent(current)) {
-    if (current->source_info().is_valid()) {
-      // Preserve the source information by replacing the current bytecode
-      // with a no op bytecode.
-      current->set_bytecode(Bytecode::kNop);
-    } else {
-      current = nullptr;
-    }
-    return current;
+void BytecodePeepholeOptimizer::TransformLdaSmiBinaryOpToBinaryOpWithSmiAction(
+    BytecodeNode* const node, const PeepholeActionAndData* action_data) {
+  DCHECK(LastIsValid());
+  DCHECK(!Bytecodes::IsJump(node->bytecode()));
+
+  if (!node->source_info().is_valid() || !last()->source_info().is_valid()) {
+    // Fused last and current into current.
+    TransformLdaSmiBinaryOpToBinaryOpWithSmi(action_data->bytecode, last(),
+                                             node);
+    // Discard last.
+    InvalidateLast();
+    // Immediately emit current node as nothing fuses with AddSmi, SubSmi, etc.

[rmcilroy, 2016/07/13 13:51:41]

Could we not do this, we might want to fuse with AddSmi, etc. in the future.

[oth, 2016/07/15 13:09:40]

How about adapting this when the future change is ready? Otherwise, there's dead
work doing the copy to last_ for something that might be needed. WDYT?

[rmcilroy, 2016/07/15 15:31:48]

I'd rather not do this since it would not be immediately apparent why these
bytecodes never get fused since this bit of code would not be near any future
change to cause AddSmi, etc. to be fused. We don't special case discarding of
last_ for other bytecodes that we currently never fuse, so I'd rather avoid this
special case if that's ok. 

[oth, 2016/07/15 17:27:26]

Done.

+    next_stage()->Write(node);
+  } else {
+    DefaultAction(node);
   }
+}
 
-  if (CanElideLast(current) && CanElideLastBasedOnSourcePosition(current)) {
-    if (last_.source_info().is_valid()) {
-      // Current can not be valid per CanElideLastBasedOnSourcePosition().
-      current->source_info().Clone(last_.source_info());
-    }
+void BytecodePeepholeOptimizer::
+    TransformLdaZeroBinaryOpToBinaryOpWithZeroAction(
+        BytecodeNode* const node, const PeepholeActionAndData* action_data) {
+  DCHECK(LastIsValid());
+  DCHECK(!Bytecodes::IsJump(node->bytecode()));
+  if (!node->source_info().is_valid() || !last()->source_info().is_valid()) {
+    // Fused last and current into current.
+    TransformLdaZeroBinaryOpToBinaryOpWithZero(action_data->bytecode, last(),
+                                               node);
+    // Discard last.
     InvalidateLast();
-    return current;
+    // Immediately emit current node as nothing fuses with AddSmi, SubSmi, etc.

[rmcilroy, 2016/07/13 13:51:41]

ditto

[oth, 2016/07/15 13:09:40]

Same here, propose only changing when needed.

[oth, 2016/07/15 17:27:25]

Done.

+    next_stage()->Write(node);
+  } else {
+    DefaultAction(node);
   }
+}
 
-  return current;
+void BytecodePeepholeOptimizer::DefaultJumpAction(
+    BytecodeNode* const node, const PeepholeActionAndData* action_data) {
+  DCHECK(LastIsValid());
+  DCHECK(Bytecodes::IsJump(node->bytecode()));
+
+  next_stage()->Write(last());
+  InvalidateLast();
 }
 
-BytecodeNode* BytecodePeepholeOptimizer::OptimizeAndEmitLast(
-    BytecodeNode* current) {
-  // Attempt optimization if there is an earlier node to optimize with.
-  if (LastIsValid()) {
-    current = Optimize(current);
-    // Only output the last node if it wasn't invalidated by the optimization.
-    if (LastIsValid()) {
-      next_stage_->Write(&last_);
-      InvalidateLast();
-    }
+void BytecodePeepholeOptimizer::UpdateLastJumpAction(
+    BytecodeNode* const node, const PeepholeActionAndData* action_data) {
+  DCHECK(!LastIsValid());
+  DCHECK(Bytecodes::IsJump(node->bytecode()));
+}
+
+void BytecodePeepholeOptimizer::ChangeJumpBytecodeAction(
+    BytecodeNode* const node, const PeepholeActionAndData* action_data) {
+  DCHECK(LastIsValid());
+  DCHECK(Bytecodes::IsJump(node->bytecode()));
+
+  next_stage()->Write(last());
+  InvalidateLast();
+  node->set_bytecode(action_data->bytecode, node->operand(0));
+}
+
+void BytecodePeepholeOptimizer::ElideLastBeforeJumpAction(
+    BytecodeNode* const node, const PeepholeActionAndData* action_data) {
+  DCHECK(LastIsValid());
+  DCHECK(Bytecodes::IsJump(node->bytecode()));
+

[rmcilroy, 2016/07/13 13:51:41]

Should we be calling CanElideLastBasedOnSourcePosition here ?

[oth, 2016/07/15 13:09:40]

Added a DCHECK as there is an assumption that this always true in code that
reaches here. 

+  if (!node->source_info().is_valid()) {
+    node->source_info().Clone(last()->source_info());
+  } else {
+    next_stage()->Write(last());
+  }
+  InvalidateLast();
+}
+
+void BytecodePeepholeOptimizer::Optimize(BytecodeNode* const node) {
+  // A single table is used for looking up peephole optimization
+  // matches as it is observed to have better performance. This is
+  // inspite of the fact that jump bytecodes and non-jump bytecodes
+  // have different processing logic, in particular a jump bytecode
+  // always needs to emit the jump via WriteJump().
+  const PeepholeActionAndData* const action_data =
+      PeepholeActionTable::Lookup(last()->bytecode(), node->bytecode());
+  switch (action_data->action) {
+#define CASE(Action)              \
+  case PeepholeAction::k##Action: \
+    Action(node, action_data);    \
+    break;
+    PEEPHOLE_ACTION_LIST(CASE)
+#undef CASE
+    default:
+      UNREACHABLE();
+      break;
   }
-  return current;
 }
 
 }  // namespace interpreter