[Interpreter] Save and restore dispatch table pointer during calls.

[Interpreter] Save and restore dispatch table pointer during calls.

Saves and restores the dispatch pointer during calls to enable the debugger to
switch the dispatch table used by a function during it's execution.

Also moves the accumulator and context nodes to be Variables so that they will
be properly merged across branches.

BUG=v8:4280, v8:4690
LOG=N

Committed: https://crrev.com/a2935d63dfc943fbf38602c5ce966e8dcb02307a
Cr-Commit-Position: refs/heads/master@{#33894}

[rmcilroy, 2016-02-10 12:33:40]

rmcilroy@chromium.org changed reviewers:
+ oth@chromium.org, yangguo@chromium.org

[rmcilroy, 2016-02-10 12:33:40]

Yang / Orion, PTAL, thanks.

[Yang, 2016-02-10 12:37:46]

lgtm. Thanks!

https://codereview.chromium.org/1684073002/diff/1/src/frames.h
File src/frames.h (right):

https://codereview.chromium.org/1684073002/diff/1/src/frames.h#newcode203
src/frames.h:203: static const int kContextFromRegisterPointer = 5 *
kPointerSize;
Can you introduce a convenience method to read and write to the dispatch table
to InterpretedFrame?

[Yang, 2016-02-10 13:59:25]

https://codereview.chromium.org/1684073002/diff/1/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):

https://codereview.chromium.org/1684073002/diff/1/src/ia32/builtins-ia32.cc#n...
src/ia32/builtins-ia32.cc:614: __ mov(ebx,
Immediate(ExternalReference::interpreter_dispatch_table_address(
Can we load this from the stack btw? We will need to do that once we have to
choose from this and the debugger dispatch table.

Or maybe I'll do this in my CL.

[rmcilroy, 2016-02-10 16:41:48]

https://codereview.chromium.org/1684073002/diff/1/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):

https://codereview.chromium.org/1684073002/diff/1/src/ia32/builtins-ia32.cc#n...
src/ia32/builtins-ia32.cc:614: __ mov(ebx,
Immediate(ExternalReference::interpreter_dispatch_table_address(
On 2016/02/10 13:59:25, Yang wrote:
> Can we load this from the stack btw? We will need to do that once we have to
> choose from this and the debugger dispatch table.
> 
> Or maybe I'll do this in my CL.

We could, but I don't think anything could change it between when we set it
above and read it here, could it?

[Yang, 2016-02-10 16:44:39]

On 2016/02/10 16:41:48, rmcilroy wrote:
> https://codereview.chromium.org/1684073002/diff/1/src/ia32/builtins-ia32.cc
> File src/ia32/builtins-ia32.cc (right):
> 
>
https://codereview.chromium.org/1684073002/diff/1/src/ia32/builtins-ia32.cc#n...
> src/ia32/builtins-ia32.cc:614: __ mov(ebx,
> Immediate(ExternalReference::interpreter_dispatch_table_address(
> On 2016/02/10 13:59:25, Yang wrote:
> > Can we load this from the stack btw? We will need to do that once we have to
> > choose from this and the debugger dispatch table.
> > 
> > Or maybe I'll do this in my CL.
> 
> We could, but I don't think anything could change it between when we set it
> above and read it here, could it?

No. it would be a convenience thing. Eventually we'd want to load the table
depending on whether there is a debug info on the shared function info. We have
the shared function info early on. Doing the check twice would be lame.

But thats just thinking ahead :)

[rmcilroy, 2016-02-11 09:00:50]

I realised I hadn't updated this to take into account your change to make the
dispatch table off-heap - PTAL.

https://codereview.chromium.org/1684073002/diff/1/src/frames.h
File src/frames.h (right):

https://codereview.chromium.org/1684073002/diff/1/src/frames.h#newcode203
src/frames.h:203: static const int kContextFromRegisterPointer = 5 *
kPointerSize;
On 2016/02/10 12:37:46, Yang wrote:
> Can you introduce a convenience method to read and write to the dispatch table
> to InterpretedFrame?

Done.

https://codereview.chromium.org/1684073002/diff/1/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):

https://codereview.chromium.org/1684073002/diff/1/src/ia32/builtins-ia32.cc#n...
src/ia32/builtins-ia32.cc:614: __ mov(ebx,
Immediate(ExternalReference::interpreter_dispatch_table_address(
On 2016/02/10 16:41:48, rmcilroy wrote:
> On 2016/02/10 13:59:25, Yang wrote:
> > > Can we load this from the stack btw? We will need to do that once we have
to
> > > choose from this and the debugger dispatch table.
> > >
> > > Or maybe I'll do this in my CL.
> 
> > We could, but I don't think anything could change it between when we set it
> > above and read it here, could it?

> No. it would be a convenience thing. Eventually we'd want to load the table
> depending on whether there is a debug info on the shared function info. We
have
> the shared function info early on. Doing the check twice would be lame.
> 
> But thats just thinking ahead :)

Ahh yes, makes perfect sense. Done.

[rmcilroy, 2016-02-11 09:01:03]

The CQ bit was checked by rmcilroy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-02-11 09:01:06]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1684073002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1684073002/20001

[commit-bot: I haz the power, 2016-02-11 09:34:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-11 09:34:42]

Dry run: This issue passed the CQ dry run.

[balazs.kilvady, 2016-02-11 10:44:00]

balazs.kilvady@imgtec.com changed reviewers:
+ balazs.kilvady@imgtec.com

[balazs.kilvady, 2016-02-11 10:44:01]

https://codereview.chromium.org/1684073002/diff/20001/src/mips/builtins-mips.cc
File src/mips/builtins-mips.cc (right):

https://codereview.chromium.org/1684073002/diff/20001/src/mips/builtins-mips....
src/mips/builtins-mips.cc:985: __ Push(zero_reg);
You could use __ Push(a0, zero_reg); to save one addu(sp, ...); instruction.

https://codereview.chromium.org/1684073002/diff/20001/src/mips64/builtins-mip...
File src/mips64/builtins-mips64.cc (right):

https://codereview.chromium.org/1684073002/diff/20001/src/mips64/builtins-mip...
src/mips64/builtins-mips64.cc:976: __ Push(a0);
You could use __ Push(a0, zero_reg); to save one daddu(sp, ...); instruction.

[oth, 2016-02-11 11:13:06]

lgtm once the tweaks in flight are done. Like the approach. Thanks.

https://codereview.chromium.org/1684073002/diff/20001/src/frames.h
File src/frames.h (right):

https://codereview.chromium.org/1684073002/diff/20001/src/frames.h#newcode748
src/frames.h:748: // Updates the current dispatch table pointer wiht
|dispatch_table|. Used by
s/wiht/with/

[rmcilroy, 2016-02-11 12:30:12]

https://codereview.chromium.org/1684073002/diff/20001/src/frames.h
File src/frames.h (right):

https://codereview.chromium.org/1684073002/diff/20001/src/frames.h#newcode748
src/frames.h:748: // Updates the current dispatch table pointer wiht
|dispatch_table|. Used by
On 2016/02/11 11:13:06, oth wrote:
> s/wiht/with/

Done.

https://codereview.chromium.org/1684073002/diff/20001/src/mips/builtins-mips.cc
File src/mips/builtins-mips.cc (right):

https://codereview.chromium.org/1684073002/diff/20001/src/mips/builtins-mips....
src/mips/builtins-mips.cc:985: __ Push(zero_reg);
On 2016/02/11 10:44:01, balazs.kilvady wrote:
> You could use __ Push(a0, zero_reg); to save one addu(sp, ...); instruction.

Good idea, done. Also pulled in the pushing of new.target to this push too and
fixed Arm to do the same. Thanks.

https://codereview.chromium.org/1684073002/diff/20001/src/mips64/builtins-mip...
File src/mips64/builtins-mips64.cc (right):

https://codereview.chromium.org/1684073002/diff/20001/src/mips64/builtins-mip...
src/mips64/builtins-mips64.cc:976: __ Push(a0);
On 2016/02/11 10:44:01, balazs.kilvady wrote:
> You could use __ Push(a0, zero_reg); to save one daddu(sp, ...); instruction.

Done.

[rmcilroy, 2016-02-11 12:39:10]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2016-02-11 12:39:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
yangguo@chromium.org, oth@chromium.org
Link to the patchset: https://codereview.chromium.org/1684073002/#ps40001
(title: "Address review comments")

[commit-bot: I haz the power, 2016-02-11 12:39:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1684073002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1684073002/40001

[commit-bot: I haz the power, 2016-02-11 13:10:51]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-11 13:11:17]

Description was changed from

==========
[Interpreter] Save and restore dispatch table pointer during calls.

Saves and restores the dispatch pointer during calls to enable the debugger to
switch the dispatch table used by a function during it's execution.

Also moves the accumulator and context nodes to be Variables so that they will
be properly merged across branches.

BUG=v8:4280,v8:4690
LOG=N
==========

to

==========
[Interpreter] Save and restore dispatch table pointer during calls.

Saves and restores the dispatch pointer during calls to enable the debugger to
switch the dispatch table used by a function during it's execution.

Also moves the accumulator and context nodes to be Variables so that they will
be properly merged across branches.

BUG=v8:4280,v8:4690
LOG=N

Committed: https://crrev.com/a2935d63dfc943fbf38602c5ce966e8dcb02307a
Cr-Commit-Position: refs/heads/master@{#33894}
==========

[commit-bot: I haz the power, 2016-02-11 13:11:18]

Patchset 3 (id:??) landed as
https://crrev.com/a2935d63dfc943fbf38602c5ce966e8dcb02307a
Cr-Commit-Position: refs/heads/master@{#33894}