[interpreter] Inline Star on dispatch for some bytecodes

src/interpreter/bytecodes.cc

 // 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.
 
 #include "src/interpreter/bytecodes.h"
 
 #include <iomanip>
 
 #include "src/base/bits.h"
 #include "src/code-stubs.h"
@@ skipping 551 matching lines @@
   case OperandType::k##Name: \
     break;
     NON_REGISTER_OPERAND_TYPE_LIST(CASE)
     REGISTER_INPUT_OPERAND_TYPE_LIST(CASE)
 #undef CASE
   }
   return false;
 }
 
 // static
+bool Bytecodes::IsStarLookahead(Bytecode bytecode, OperandScale operand_scale) {
+  if (operand_scale == OperandScale::kSingle) {
+    switch (bytecode) {
+      case Bytecode::kLdaZero:

[oth, 2016/07/18 08:05:40]

Is there data driving the selected bytecodes here?

Naively, it might seem that all binary operators would benefit from the
optimization proposed here (?) and perhaps other bytecodes too. What happens if
this done for all bytecodes?

[klaasb, 2016/07/18 09:48:54]

Some experiments are in the earlier patchsets (although code has been refactored
since then). Octane shows less improvement when this is enabled for more
bytecodes and some of it regresses more.

I've gathered data from speedometer, verge and facebook that show mostly the
same set of byte codes often dispatching to Star and a similar list for Octane
which shows a set with quite some differences (more Shift*Smi, Bitwise*Smi,
StaKeyed*Sloppy). Most bytecodes don't dispatch to Star all that often, mostly
an order of magnitude less than these.

These are mostly the set that all agree on within the Top 20 bytecodes and
mostly arithmetic bytecodes.
From the benchmarks and dispatch data I don't think it would improve if done for
all bytecodes.

Should I put a comment here, so others know what to look for when adding a
Bytecode here?

+      case Bytecode::kLdaSmi:
+      case Bytecode::kLdaNull:
+      case Bytecode::kLdaConstant:
+      case Bytecode::kMov:

[rmcilroy, 2016/07/18 11:07:51]

kMov seems an unusual one since it doesn't update the accumulator (so I'm not
sure what the Star would be doing here) - Could you check this one.

[klaasb, 2016/07/18 15:15:28]

I thought about that too. It is the top 10-20 for all benchmarks and websites I
looked at, though. Sometimes even higher up than LdaZero, Sub, SubSmi and always
higher up than New and higher than Dec except on Octane. 11th most source of
dispatch in Octane.

I can have look whether it's storing to the same registers Mov is using, or
whether it's just coincidence.

+      case Bytecode::kAdd:
+      case Bytecode::kSub:
+      case Bytecode::kMul:
+      case Bytecode::kAddSmi:
+      case Bytecode::kSubSmi:
+      case Bytecode::kInc:
+      case Bytecode::kDec:
+      case Bytecode::kCall:
+      case Bytecode::kNew:
+        return true;
+      default:
+        return false;
+    }
+  }
+  return false;
+}
+
+// static
 int Bytecodes::GetNumberOfRegistersRepresentedBy(OperandType operand_type) {
   switch (operand_type) {
     case OperandType::kMaybeReg:
     case OperandType::kReg:
     case OperandType::kRegOut:
       return 1;
     case OperandType::kRegPair:
     case OperandType::kRegOutPair:
       return 2;
     case OperandType::kRegOutTriple:
@@ skipping 363 matching lines @@
   if (!FLAG_always_opt && !FLAG_prepare_always_opt &&
       pretenure == NOT_TENURED && is_function_scope) {
     result |= FastNewClosureBit::encode(true);
   }
   return result;
 }
 
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8