[Interpreter] Adds implementation of bytecode graph builder for LoadICSloppy/Strict

src/compiler/bytecode-graph-builder.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/compiler/bytecode-graph-builder.h"
 
 #include "src/compiler/linkage.h"
 #include "src/compiler/operator-properties.h"
 #include "src/interpreter/bytecode-array-iterator.h"
 
@@ skipping 80 matching lines @@
 bool BytecodeGraphBuilder::Environment::IsMarkedAsUnreachable() const {
   return GetControlDependency()->opcode() == IrOpcode::kDead;
 }
 
 
 void BytecodeGraphBuilder::Environment::MarkAsUnreachable() {
   UpdateControlDependency(builder()->jsgraph()->Dead());
 }
 
 
+class BytecodeGraphBuilder::FrameStateBeforeAndAfter {
+ public:
+  // TODO(mythria) remove explicit when there are more parameters for
+  // constructor.

[rmcilroy, 2015/11/04 14:27:37]

no need for this comment

[mythria, 2015/11/09 12:08:37]

Done.

+  explicit FrameStateBeforeAndAfter(BytecodeGraphBuilder* builder)
+      : builder_(builder) {
+    // TODO(mythria) Replace with the actual frame state. Current

[rmcilroy, 2015/11/04 14:27:37]

/s/TODO(mythria)/TODO(mythria):/ (add the colon after the todo - throughout).

[mythria, 2015/11/09 12:08:37]

Done.

+    // implementation introduces empty frame state.
+    frame_state_before_ = builder_->jsgraph()->EmptyFrameState();
+  }
+

[rmcilroy, 2015/11/04 14:27:37]

nit - remove extra newline (only a single line between class definition
elements.

[mythria, 2015/11/09 12:08:37]

Done.

+
+  void AddToNode(Node* node) {

[mythria, 2015/11/04 12:19:14]

In the AstGraphBuilder, this function expects two more parameters for
constructing the frame state. I did not include them in this implementation. I
am not sure if we need them or not. Actually, I did not completely understand
what they are. I could look up and update the implementation/comment.

+    int count = OperatorProperties::GetFrameStateInputCount(node->op());
+    DCHECK_LE(count, 2);
+    if (count >= 1) {
+      // Add the frame state for after the operation.
+      DCHECK_EQ(IrOpcode::kDead,
+                NodeProperties::GetFrameStateInput(node, 0)->opcode());
+      // TODO(mythria) Replace with the actual frame state. Current

[rmcilroy, 2015/11/04 14:27:36]

nit - newline above

[mythria, 2015/11/09 12:08:37]

Done.

+      // implementation introduces empty frame state.
+      NodeProperties::ReplaceFrameStateInput(
+          node, 0, builder_->jsgraph()->EmptyFrameState());
+    }
+    if (count >= 2) {
+      // Add the frame state for before the operation.
+      DCHECK_EQ(IrOpcode::kDead,
+                NodeProperties::GetFrameStateInput(node, 1)->opcode());
+      NodeProperties::ReplaceFrameStateInput(node, 1, frame_state_before_);
+    }
+  }
+
+ private:
+  BytecodeGraphBuilder* builder_;
+  Node* frame_state_before_;
+};
+
+
 BytecodeGraphBuilder::BytecodeGraphBuilder(Zone* local_zone,
                                            CompilationInfo* compilation_info,
                                            JSGraph* jsgraph)
     : local_zone_(local_zone),
       info_(compilation_info),
       jsgraph_(jsgraph),
       input_buffer_size_(0),
       input_buffer_(nullptr),
       exit_controls_(local_zone) {
   bytecode_array_ = handle(info()->shared_info()->bytecode_array());
 }
 
 
 Node* BytecodeGraphBuilder::GetFunctionContext() {
   if (!function_context_.is_set()) {
     // Parameter (arity + 1) is special for the outer context of the function
     const Operator* op =
         common()->Parameter(bytecode_array()->parameter_count(), "%context");
     Node* node = NewNode(op, graph()->start());
     function_context_.set(node);
   }
   return function_context_.get();
 }
 
 
+Node* BytecodeGraphBuilder::GetFunctionClosure() {
+  if (!function_closure_.is_set()) {
+    const Operator* op = common()->Parameter(
+        Linkage::kJSFunctionCallClosureParamIndex, "%closure");
+    Node* node = NewNode(op, graph()->start());
+    function_closure_.set(node);
+  }
+  return function_closure_.get();
+}
+
+
+Node* BytecodeGraphBuilder::BuildLoadObjectField(Node* object, int offset) {
+  return NewNode(jsgraph()->machine()->Load(kMachAnyTagged), object,
+                 jsgraph()->IntPtrConstant(offset - kHeapObjectTag));
+}
+
+
+Node* BytecodeGraphBuilder::BuildLoadImmutableObjectField(Node* object,
+                                                          int offset) {
+  return graph()->NewNode(jsgraph()->machine()->Load(kMachAnyTagged), object,
+                          jsgraph()->IntPtrConstant(offset - kHeapObjectTag),
+                          graph()->start(), graph()->start());
+}
+
+
+Node* BytecodeGraphBuilder::BuildLoadFeedbackVector() {
+  if (!feedback_vector_.is_set()) {
+    Node* closure = GetFunctionClosure();
+    Node* shared = BuildLoadImmutableObjectField(
+        closure, JSFunction::kSharedFunctionInfoOffset);
+    Node* vector = BuildLoadImmutableObjectField(
+        shared, SharedFunctionInfo::kFeedbackVectorOffset);
+    feedback_vector_.set(vector);
+  }
+  return feedback_vector_.get();
+}
+
+
+VectorSlotPair BytecodeGraphBuilder::CreateVectorSlotPair(
+    FeedbackVectorSlot slot) {
+  return VectorSlotPair(handle(info()->shared_info()->feedback_vector()), slot);
+}
+
+
+// TODO(mythria) This implementation introduces empty frame state. Replace with
+// the correct frame state.
+void BytecodeGraphBuilder::PrepareFrameState(Node* node) {

[mythria, 2015/11/04 12:19:14]

In the AstGraphBuilder, this function expects two more parameters. Same as
FrameStateBeforeAndAfter::AddToNode 

+  int count = OperatorProperties::GetFrameStateInputCount(node->op());
+  if (count > 0) {
+    DCHECK_EQ(1, count);
+    DCHECK_EQ(IrOpcode::kDead,
+              NodeProperties::GetFrameStateInput(node, 0)->opcode());
+    NodeProperties::ReplaceFrameStateInput(node, 0,
+                                           jsgraph()->EmptyFrameState());
+  }
+}
+
+
 bool BytecodeGraphBuilder::CreateGraph(bool stack_check) {
   // Set up the basic structure of the graph. Outputs for {Start} are
   // the formal parameters (including the receiver) plus context and
   // closure.
 
   // The additional count items are for the context and closure.
   int actual_parameter_count = bytecode_array()->parameter_count() + 2;
   graph()->SetStart(graph()->NewNode(common()->Start(actual_parameter_count)));
 
   Environment env(this, bytecode_array()->register_count(),
@@ skipping 174 matching lines @@
 
 
 void BytecodeGraphBuilder::VisitStaContextSlot(
     const interpreter::BytecodeArrayIterator& iterator) {
   UNIMPLEMENTED();
 }
 
 
 void BytecodeGraphBuilder::VisitLoadICSloppy(
     const interpreter::BytecodeArrayIterator& iterator) {
-  UNIMPLEMENTED();
+  FrameStateBeforeAndAfter states(this);

[rmcilroy, 2015/11/04 14:27:37]

Add a DCHECK(is_sloppy(language_mode()) (and similarly is_strict() for the
strict variants).

[mythria, 2015/11/09 12:08:37]

Done.

+  Node* object = environment()->LookupRegister(iterator.GetRegisterOperand(0));
+  Handle<Name> name =
+      Handle<Name>::cast(iterator.GetConstantForIndexOperand(1));
+  FeedbackVectorSlot slot =
+      info()->feedback_vector()->ToSlot(iterator.GetIndexOperand(2));
+  VectorSlotPair feedback = CreateVectorSlotPair(slot);

[rmcilroy, 2015/11/04 14:27:37]

Could you create a helper for the 
  FeedbackVectorSlot slot = 
info()->feedback_vector()->ToSlot(iterator.GetIndexOperand(<operand>));
  return CreateVectorSlotPair(slot);

I have a feeling we will be doing this a lot.

[mythria, 2015/11/09 12:08:37]

Done.

+  const Operator* op = javascript()->LoadNamed(language_mode(), name, feedback);

[rmcilroy, 2015/11/04 14:27:37]

nit - newline above

+  Node* node = NewNode(op, object, BuildLoadFeedbackVector());
+  states.AddToNode(node);
+  environment()->BindAccumulator(node);

[rmcilroy, 2015/11/04 14:27:37]

Could you pull this out into a BuildNamedLoad helper function, and have
VisitLoadICStrict, VisitLoadICSloppyWide and VisitLoadICStrictWide also call
this function.

[mythria, 2015/11/09 12:08:37]

Done.

 }
 
 
 void BytecodeGraphBuilder::VisitLoadICStrict(
     const interpreter::BytecodeArrayIterator& iterator) {
   UNIMPLEMENTED();
 }
 
 
 void BytecodeGraphBuilder::VisitKeyedLoadICSloppy(
@@ skipping 117 matching lines @@
 
 
 void BytecodeGraphBuilder::VisitCreateArrayLiteral(
     const interpreter::BytecodeArrayIterator& iterator) {
   UNIMPLEMENTED();
 }
 
 
 void BytecodeGraphBuilder::VisitCreateObjectLiteral(
     const interpreter::BytecodeArrayIterator& iterator) {
-  UNIMPLEMENTED();
+  Node* closure = GetFunctionClosure();
+  Node* literals_array =
+      BuildLoadObjectField(closure, JSFunction::kLiteralsOffset);
+  Node* literal_index = jsgraph()->Constant(iterator.GetIndexOperand(0));
+  Node* constants = environment()->LookupAccumulator();
+  const Operator* op =
+      javascript()->CreateLiteralObject(iterator.GetImmediateOperand(1));
+  Node* literal = NewNode(op, literals_array, literal_index, constants);
+  PrepareFrameState(literal);
+  environment()->BindAccumulator(literal);

[rmcilroy, 2015/11/04 14:27:37]

Could you remove this and do it in a separate CL please. You can modify the test
to pass an object to the function under test so that you don't need support for
CreateObjectLiteral for the test.

Also, please move BuildLoadObjectField to the other CL since this is only used
in this visitor.

[mythria, 2015/11/09 12:08:37]

Done.

 }
 
 
 void BytecodeGraphBuilder::VisitCall(
     const interpreter::BytecodeArrayIterator& iterator) {
   UNIMPLEMENTED();
 }
 
 
 void BytecodeGraphBuilder::VisitCallRuntime(
@@ skipping 423 matching lines @@
 
 void BytecodeGraphBuilder::UpdateControlDependencyToLeaveFunction(Node* exit) {
   if (environment()->IsMarkedAsUnreachable()) return;
   environment()->MarkAsUnreachable();
   exit_controls_.push_back(exit);
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8