Revert of [Interpreter] Collect type feedback for 'new' in the byteco…

…de handler (patchset #6 id:100001 of https://codereview.chromium.org/2153433002/ )

Reason for revert:
This cl causes a large regression in octane (https://chromeperf.appspot.com/group_report?bug_id=629503). I have to investigate the reason before I can reland this.

Original issue's description:
> [Interpreter] Collect type feedback for 'new' in the bytecode handler
>
> Collect type feedback in the bytecode handler for 'new' bytecode. The
> current implementation does not collect allocation site feedback.
>
> BUG=v8:4280, v8:4780
> LOG=N
>
> Committed: https://crrev.com/1eadc76419b323fb2e55ae9953142f801704aa59
> Cr-Commit-Position: refs/heads/master@{#37862}

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

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

src/builtins/arm/builtins-arm.cc

@@ -1206,8 +1206,7 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl(
 }
 
 // static
-void Builtins::Generate_InterpreterPushArgsAndConstructImpl(
-    MacroAssembler* masm, CallableType construct_type) {
+void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) {
   // ----------- S t a t e -------------
   // -- r0 : argument count (not including receiver)
   // -- r3 : new target
@@ -1226,16 +1225,8 @@ void Builtins::Generate_InterpreterPushArgsAndConstructImpl(
   // Push the arguments.
   Generate_InterpreterPushArgs(masm, r2, r4, r5);
 
-  if (construct_type == CallableType::kJSFunction) {
-    // TODO(mythria): Change this when allocation site feedback is available.
-    // ConstructFunction initializes allocation site to undefined.
-    __ Jump(masm->isolate()->builtins()->ConstructFunction(),
-            RelocInfo::CODE_TARGET);
-  } else {
-    DCHECK_EQ(construct_type, CallableType::kAny);
-    // Call the constructor with r0, r1, and r3 unmodified.
-    __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET);
-  }
+  // Call the constructor with r0, r1, and r3 unmodified.
+  __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET);
 }
 
 void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) {

src/builtins/arm64/builtins-arm64.cc

@@ -1213,8 +1213,7 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl(
 }
 
 // static
-void Builtins::Generate_InterpreterPushArgsAndConstructImpl(
-    MacroAssembler* masm, CallableType construct_type) {
+void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) {
   // ----------- S t a t e -------------
   // -- x0 : argument count (not including receiver)
   // -- x3 : new target
@@ -1245,16 +1244,8 @@ void Builtins::Generate_InterpreterPushArgsAndConstructImpl(
   __ Cmp(x6, x4);
   __ B(gt, &loop_header);
 
-  if (construct_type == CallableType::kJSFunction) {
-    // TODO(mythria): Change this when allocation site feedback is available.
-    // ConstructFunction initializes allocation site to undefined.
-    __ Jump(masm->isolate()->builtins()->ConstructFunction(),
-            RelocInfo::CODE_TARGET);
-  } else {
-    DCHECK_EQ(construct_type, CallableType::kAny);
-    // Call the constructor with x0, x1, and x3 unmodified.
-    __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET);
-  }
+  // Call the constructor with x0, x1, and x3 unmodified.
+  __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET);
 }
 
 void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) {

src/builtins/builtins.cc

@@ -4733,18 +4733,6 @@ Handle<Code> Builtins::InterpreterPushArgsAndCall(TailCallMode tail_call_mode,
   return Handle<Code>::null();
 }
 
-Handle<Code> Builtins::InterpreterPushArgsAndConstruct(
-    CallableType function_type) {
-  switch (function_type) {
-    case CallableType::kJSFunction:
-      return InterpreterPushArgsAndConstructFunction();
-    case CallableType::kAny:
-      return InterpreterPushArgsAndConstruct();
-  }
-  UNREACHABLE();
-  return Handle<Code>::null();
-}
-
 namespace {
 
 class RelocatableArguments : public BuiltinArguments, public Relocatable {
@@ -5850,16 +5838,6 @@ void Builtins::Generate_InterpreterPushArgsAndTailCallFunction(
                                                  CallableType::kJSFunction);
 }
 
-void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) {
-  return Generate_InterpreterPushArgsAndConstructImpl(masm, CallableType::kAny);
-}
-
-void Builtins::Generate_InterpreterPushArgsAndConstructFunction(
-    MacroAssembler* masm) {
-  return Generate_InterpreterPushArgsAndConstructImpl(
-      masm, CallableType::kJSFunction);
-}
-
 void Builtins::Generate_MathMax(MacroAssembler* masm) {
   Generate_MathMaxMin(masm, MathMaxMinKind::kMax);
 }

src/builtins/builtins.h

@@ -129,7 +129,6 @@ namespace internal {
   ASM(InterpreterPushArgsAndCall)                                            \
   ASM(InterpreterPushArgsAndTailCall)                                        \
   ASM(InterpreterPushArgsAndConstruct)                                       \
-  ASM(InterpreterPushArgsAndConstructFunction)                               \
   ASM(InterpreterEnterBytecodeDispatch)                                      \
                                                                              \
   /* Code life-cycle */                                                      \
@@ -558,7 +557,6 @@ class Builtins {
   Handle<Code> InterpreterPushArgsAndCall(
       TailCallMode tail_call_mode,
       CallableType function_type = CallableType::kAny);
-  Handle<Code> InterpreterPushArgsAndConstruct(CallableType function_type);
 
   Code* builtin(Name name) {
     // Code::cast cannot be used here since we access builtins
@@ -600,9 +598,6 @@ class Builtins {
       MacroAssembler* masm, TailCallMode tail_call_mode,
       CallableType function_type);
 
-  static void Generate_InterpreterPushArgsAndConstructImpl(
-      MacroAssembler* masm, CallableType function_type);
-
   static void Generate_DatePrototype_GetField(MacroAssembler* masm,
                                               int field_index);
 

src/builtins/ia32/builtins-ia32.cc

@@ -758,8 +758,7 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl(
 }
 
 // static
-void Builtins::Generate_InterpreterPushArgsAndConstructImpl(
-    MacroAssembler* masm, CallableType construct_type) {
+void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) {
   // ----------- S t a t e -------------
   //  -- eax : the number of arguments (not including the receiver)
   //  -- edx : the new target
@@ -791,17 +790,8 @@ void Builtins::Generate_InterpreterPushArgsAndConstructImpl(
   // Re-push return address.
   __ Push(ecx);
 
-  if (construct_type == CallableType::kJSFunction) {
-    // TODO(mythria): Change this when allocation site feedback is available.
-    // ConstructFunction initializes allocation site to undefined.
-    __ Jump(masm->isolate()->builtins()->ConstructFunction(),
-            RelocInfo::CODE_TARGET);
-  } else {
-    DCHECK_EQ(construct_type, CallableType::kAny);
-
-    // Call the constructor with unmodified eax, edi, ebi values.
-    __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET);
-  }
+  // Call the constructor with unmodified eax, edi, ebi values.
+  __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET);
 }
 
 void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) {

src/builtins/mips/builtins-mips.cc

@@ -1198,8 +1198,7 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl(
 }
 
 // static
-void Builtins::Generate_InterpreterPushArgsAndConstructImpl(
-    MacroAssembler* masm, CallableType construct_type) {
+void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) {
   // ----------- S t a t e -------------
   // -- a0 : argument count (not including receiver)
   // -- a3 : new target
@@ -1224,16 +1223,8 @@ void Builtins::Generate_InterpreterPushArgsAndConstructImpl(
   __ bind(&loop_check);
   __ Branch(&loop_header, gt, a2, Operand(t0));
 
-  if (construct_type == CallableType::kJSFunction) {
-    // TODO(mythria): Change this when allocation site feedback is available.
-    // ConstructFunction initializes allocation site to undefined.
-    __ Jump(masm->isolate()->builtins()->ConstructFunction(),
-            RelocInfo::CODE_TARGET);
-  } else {
-    DCHECK_EQ(construct_type, CallableType::kAny);
-    // Call the constructor with a0, a1, and a3 unmodified.
-    __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET);
-  }
+  // Call the constructor with a0, a1, and a3 unmodified.
+  __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET);
 }
 
 void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) {

src/builtins/mips64/builtins-mips64.cc

@@ -1190,8 +1190,7 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl(
 }
 
 // static
-void Builtins::Generate_InterpreterPushArgsAndConstructImpl(
-    MacroAssembler* masm, CallableType construct_type) {
+void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) {
   // ----------- S t a t e -------------
   // -- a0 : argument count (not including receiver)
   // -- a3 : new target
@@ -1216,16 +1215,8 @@ void Builtins::Generate_InterpreterPushArgsAndConstructImpl(
   __ bind(&loop_check);
   __ Branch(&loop_header, gt, a2, Operand(t0));
 
-  if (construct_type == CallableType::kJSFunction) {
-    // TODO(mythria): Change this when allocation site feedback is available.
-    // ConstructFunction initializes allocation site to undefined.
-    __ Jump(masm->isolate()->builtins()->ConstructFunction(),
-            RelocInfo::CODE_TARGET);
-  } else {
-    DCHECK_EQ(construct_type, CallableType::kAny);
-    // Call the constructor with a0, a1, and a3 unmodified.
-    __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET);
-  }
+  // Call the constructor with a0, a1, and a3 unmodified.
+  __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET);
 }
 
 void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) {

src/builtins/x64/builtins-x64.cc

@@ -840,8 +840,7 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl(
 }
 
 // static
-void Builtins::Generate_InterpreterPushArgsAndConstructImpl(
-    MacroAssembler* masm, CallableType construct_type) {
+void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) {
   // ----------- S t a t e -------------
   //  -- rax : the number of arguments (not including the receiver)
   //  -- rdx : the new target (either the same as the constructor or
@@ -863,16 +862,8 @@ void Builtins::Generate_InterpreterPushArgsAndConstructImpl(
   // Push return address in preparation for the tail-call.
   __ PushReturnAddressFrom(kScratchRegister);
 
-  if (construct_type == CallableType::kJSFunction) {
-    // TODO(mythria): Change this when allocation site feedback is available.
-    // ConstructFunction initializes allocation site to undefined.
-    __ Jump(masm->isolate()->builtins()->ConstructFunction(),
-            RelocInfo::CODE_TARGET);
-  } else {
-    DCHECK_EQ(construct_type, CallableType::kAny);
-    // Call the constructor (rax, rdx, rdi passed on).
-    __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET);
-  }
+  // Call the constructor (rax, rdx, rdi passed on).
+  __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET);
 }
 
 void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) {

src/code-factory.cc

@@ -581,11 +581,9 @@ Callable CodeFactory::InterpreterPushArgsAndCall(Isolate* isolate,
 
 
 // static
-Callable CodeFactory::InterpreterPushArgsAndConstruct(
-    Isolate* isolate, CallableType function_type) {
-  return Callable(
-      isolate->builtins()->InterpreterPushArgsAndConstruct(function_type),
-      InterpreterPushArgsAndConstructDescriptor(isolate));
+Callable CodeFactory::InterpreterPushArgsAndConstruct(Isolate* isolate) {
+  return Callable(isolate->builtins()->InterpreterPushArgsAndConstruct(),
+                  InterpreterPushArgsAndConstructDescriptor(isolate));
 }
 
 

src/code-factory.h

@@ -154,8 +154,7 @@ class CodeFactory final {
   static Callable InterpreterPushArgsAndCall(
       Isolate* isolate, TailCallMode tail_call_mode,
       CallableType function_type = CallableType::kAny);
-  static Callable InterpreterPushArgsAndConstruct(
-      Isolate* isolate, CallableType function_type = CallableType::kAny);
+  static Callable InterpreterPushArgsAndConstruct(Isolate* isolate);
   static Callable InterpreterCEntry(Isolate* isolate, int result_size = 1);
 };
 

src/compiler/bytecode-graph-builder.cc

@@ -1060,17 +1060,12 @@ void BytecodeGraphBuilder::VisitNew() {
   interpreter::Register callee_reg = bytecode_iterator().GetRegisterOperand(0);
   interpreter::Register first_arg = bytecode_iterator().GetRegisterOperand(1);
   size_t arg_count = bytecode_iterator().GetRegisterCountOperand(2);
-  // Slot index of 0 is used indicate no feedback slot is available. Assert
-  // the assumption that slot index 0 is never a valid feedback slot.
-  STATIC_ASSERT(TypeFeedbackVector::kReservedIndexCount > 0);
-  VectorSlotPair feedback =
-      CreateVectorSlotPair(bytecode_iterator().GetIndexOperand(3));
 
   Node* new_target = environment()->LookupAccumulator();
   Node* callee = environment()->LookupRegister(callee_reg);
-
-  const Operator* call =
-      javascript()->CallConstruct(static_cast<int>(arg_count) + 2, feedback);
+  // TODO(turbofan): Pass the feedback here.
+  const Operator* call = javascript()->CallConstruct(
+      static_cast<int>(arg_count) + 2, VectorSlotPair());
   Node* value = ProcessCallNewArguments(call, callee, new_target, first_arg,
                                         arg_count + 2);
   environment()->BindAccumulator(value, &states);

src/interpreter/bytecode-array-builder.cc

@@ -569,15 +569,13 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::Call(Register callable,
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::New(Register constructor,
                                                 Register first_arg,
-                                                size_t arg_count,
-                                                int feedback_slot_id) {
+                                                size_t arg_count) {
   if (!first_arg.is_valid()) {
     DCHECK_EQ(0u, arg_count);
     first_arg = Register(0);
   }
   Output(Bytecode::kNew, RegisterOperand(constructor),
-         RegisterOperand(first_arg), UnsignedOperand(arg_count),
-         UnsignedOperand(feedback_slot_id));
+         RegisterOperand(first_arg), UnsignedOperand(arg_count));
   return *this;
 }
 

src/interpreter/bytecode-array-builder.h

@@ -172,7 +172,7 @@ class BytecodeArrayBuilder final : public ZoneObject {
   // consecutive arguments starting at |first_arg| for the constuctor
   // invocation.
   BytecodeArrayBuilder& New(Register constructor, Register first_arg,
-                            size_t arg_count, int feedback_slot);
+                            size_t arg_count);
 
   // Call the runtime function with |function_id|. The first argument should be
   // in |first_arg| and all subsequent arguments should be in registers

src/interpreter/bytecode-generator.cc

@@ -2527,7 +2527,7 @@ void BytecodeGenerator::VisitCall(Call* expr) {
   if (expr->CallFeedbackICSlot().IsInvalid()) {
     DCHECK(call_type == Call::POSSIBLY_EVAL_CALL);
     // Valid type feedback slots can only be greater than kReservedIndexCount.
-    // We use 0 to indicate an invalid slot id. Statically assert that 0 cannot
+    // We use 0 to indicate an invalid slot it. Statically assert that 0 cannot
     // be a valid slot id.
     STATIC_ASSERT(TypeFeedbackVector::kReservedIndexCount > 0);
     feedback_slot_index = 0;
@@ -2562,13 +2562,7 @@ void BytecodeGenerator::VisitCallSuper(Call* expr) {
 
   // Call construct.
   builder()->SetExpressionPosition(expr);
-  // Valid type feedback slots can only be greater than kReservedIndexCount.
-  // Assert that 0 cannot be valid a valid slot id.
-  STATIC_ASSERT(TypeFeedbackVector::kReservedIndexCount > 0);
-  // Type feedback is not necessary for super constructor calls. The type
-  // information can be inferred in most cases. Slot id 0 indicates type
-  // feedback is not required.
-  builder()->New(constructor, first_arg, args->length(), 0);
+  builder()->New(constructor, first_arg, args->length());
   execution_result()->SetResultInAccumulator();
 }
 
@@ -2585,8 +2579,7 @@ void BytecodeGenerator::VisitCallNew(CallNew* expr) {
   // constructor for CallNew.
   builder()
       ->LoadAccumulatorWithRegister(constructor)
-      .New(constructor, first_arg, args->length(),
-           feedback_index(expr->CallNewFeedbackSlot()));
+      .New(constructor, first_arg, args->length());
   execution_result()->SetResultInAccumulator();
 }
 

src/interpreter/bytecodes.h

@@ -197,7 +197,7 @@ namespace interpreter {
                                                                                \
   /* New operator */                                                           \
   V(New, AccumulatorUse::kReadWrite, OperandType::kReg,                        \
-    OperandType::kMaybeReg, OperandType::kRegCount, OperandType::kIdx)         \
+    OperandType::kMaybeReg, OperandType::kRegCount)                            \
                                                                                \
   /* Test Operators */                                                         \
   V(TestEqual, AccumulatorUse::kReadWrite, OperandType::kReg)                  \