[IC] IC::GetSharedFunctionInfo does not need to skip bytecode handler frames.

src/ic/ic.cc

Index: src/ic/ic.cc
diff --git a/src/ic/ic.cc b/src/ic/ic.cc
index d9fb716ab26b2756300b4deb274de9a062427b4d..fbc11fb5db4bc7808e7f0ae141140d673ac035a7 100644
--- a/src/ic/ic.cc
+++ b/src/ic/ic.cc
@@ -192,12 +192,15 @@ IC::IC(FrameDepth depth, Isolate* isolate, FeedbackNexus* nexus)
   // function's frame. Check if the there is an additional frame, and if there
   // is skip this frame. However, the pc should not be updated. The call to
   // ICs happen from bytecode handlers.
+  // TODO(rmcilroy): Remove this once bytecode handlers don't need a frame.

[mythria, 2016/10/31 15:47:37]

Do we need this TODO? since we need to call into runtime for this handler, is it
possible to avoid a frame?

[rmcilroy, 2016/11/01 13:01:02]

Yeah I had plans to avoid needing to build the frame, but it's probably not
worth it. Could you just remove this TODO please.

[mythria, 2016/11/01 15:55:53]

Done.

   Object* frame_type =
       Memory::Object_at(fp + TypedFrameConstants::kFrameTypeOffset);
   if (frame_type == Smi::FromInt(StackFrame::STUB)) {
     fp = Memory::Address_at(fp + TypedFrameConstants::kCallerFPOffset);
   }
+
   fp_ = fp;
+

[rmcilroy, 2016/11/01 13:01:02]

Intended changes?

[mythria, 2016/11/01 15:55:53]

No. Removed them. Done.

   if (FLAG_enable_embedded_constant_pool) {
     constant_pool_address_ = constant_pool;
   }
@@ -250,10 +253,10 @@ SharedFunctionInfo* IC::GetSharedFunctionInfo() const {
   // corresponding to the frame.
   StackFrameIterator it(isolate());
   while (it.frame()->fp() != this->fp()) it.Advance();
-  if (it.frame()->type() == StackFrame::STUB) {
-    // We might need to advance over bytecode handler frame for Ignition.
-    it.Advance();
-  }
+  // For ignition, bytecode handlers build a stub frame. This frame should be
+  // skipped in the constructor. DCHECK to be sure we skipped the frame.
+  DCHECK(it.frame()->type() != StackFrame::STUB);

[mythria, 2016/10/31 15:47:37]

I wanted to add a check to see if it is the correct type, but that has a lot of
frame types: Interpreted, optimized, javascript, builtin and may be
ArgumentsAdaptor.

[rmcilroy, 2016/11/01 13:01:02]

Hmm, we could probably just remove this - I'm not sure we need to check it isn't
a STUB frame.

[mythria, 2016/11/01 15:55:54]

Removed it. I wasn't very happy with that check either. 

+
   JavaScriptFrame* frame = JavaScriptFrame::cast(it.frame());
   // Find the function on the stack and both the active code for the
   // function and the original code.