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

src/compiler/bytecode-graph-builder.cc

Index: src/compiler/bytecode-graph-builder.cc
diff --git a/src/compiler/bytecode-graph-builder.cc b/src/compiler/bytecode-graph-builder.cc
index 5c81906a7e19bd664442c2e6e93a25902c03b8b9..f9cae6567e0034b81b157f8e837397a5af9e23df 100644
--- a/src/compiler/bytecode-graph-builder.cc
+++ b/src/compiler/bytecode-graph-builder.cc
@@ -98,6 +98,44 @@ void BytecodeGraphBuilder::Environment::MarkAsUnreachable() {
 }
 
 
+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)
@@ -123,6 +161,64 @@ Node* BytecodeGraphBuilder::GetFunctionContext() {
 }
 
 
+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
@@ -317,7 +413,17 @@ void BytecodeGraphBuilder::VisitStaContextSlot(
 
 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.

 }
 
 
@@ -455,7 +561,16 @@ void BytecodeGraphBuilder::VisitCreateArrayLiteral(
 
 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.

 }