[interpreter] Move temporary register allocator into own file.

[interpreter] Move temporary register allocator into own file.

Moves the temporary register allocator out of the bytecode array
builder into TemporaryRegisterAllocator class and adds unittests.
Particular must be taken around the translation window boundary
motivating the addition of tests.

Also adds a Clear() method to IdentityMap() which is called by
the destructor. This allows classes to hold an IdentityMap if
they are zone allocated. Classes must call Clear() before the zone
is re-cycled or face v8 heap corruption.

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

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

[oth, 2016-02-01 14:49:25]

oth@chromium.org changed reviewers:
+ mstarzinger@chromium.org, mythria@chromium.org, rmcilroy@chromium.org

[oth, 2016-02-01 14:49:25]

mstarzinger@chromium.org: Please review changes in src/identity-map.{h,cc}

mythria, rmcilroy: Please review changes in the interpreter and the tests. There
is some minor renaming here and changes to
TemporaryRegisterAllocator::BorrowNotInRange().

Thanks

[Michael Starzinger, 2016-02-01 16:23:38]

LGTM on "identity-map", didn't look at the rest.

[rmcilroy, 2016-02-02 10:53:28]

lgtm

https://codereview.chromium.org/1651133002/diff/80001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-iterator.cc (right):

https://codereview.chromium.org/1651133002/diff/80001/src/interpreter/bytecod...
src/interpreter/bytecode-array-iterator.cc:102: reg = Register::invalid_value();
break;

https://codereview.chromium.org/1651133002/diff/80001/src/interpreter/bytecod...
src/interpreter/bytecode-array-iterator.cc:112: OperandType::kMaybeReg16)));
nit - could you pull out a helper for IsMaybeRegisterOperandType for this check?

https://codereview.chromium.org/1651133002/diff/80001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/1651133002/diff/80001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:564:
scope()->MaxNestedContextChainLength(), scope()->num_stack_slots()));
optional nit - could we do this before allocating ContextScope / ControlScopes?
Seems safer somehow...

[oth, 2016-02-02 11:20:24]

thanks, done.

https://codereview.chromium.org/1651133002/diff/80001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-iterator.cc (right):

https://codereview.chromium.org/1651133002/diff/80001/src/interpreter/bytecod...
src/interpreter/bytecode-array-iterator.cc:102: reg = Register::invalid_value();
On 2016/02/02 10:53:28, rmcilroy wrote:
> break;

Done.

https://codereview.chromium.org/1651133002/diff/80001/src/interpreter/bytecod...
src/interpreter/bytecode-array-iterator.cc:112: OperandType::kMaybeReg16)));
On 2016/02/02 10:53:28, rmcilroy wrote:
> nit - could you pull out a helper for IsMaybeRegisterOperandType for this
check?

Done. Also IsRegisterCountOperandType used just below.

https://codereview.chromium.org/1651133002/diff/80001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/1651133002/diff/80001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:564:
scope()->MaxNestedContextChainLength(), scope()->num_stack_slots()));
On 2016/02/02 10:53:28, rmcilroy wrote:
> optional nit - could we do this before allocating ContextScope /
ControlScopes?
> Seems safer somehow...

Done.

[mythria, 2016-02-02 11:47:19]

lgtm

[oth, 2016-02-02 14:08:57]

The CQ bit was checked by oth@chromium.org

[oth, 2016-02-02 14:08:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/1651133002/#ps100001
(title: "Incorporate review comments from rmcilroy.")

[commit-bot: I haz the power, 2016-02-02 14:10:15]

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

[commit-bot: I haz the power, 2016-02-02 14:31:47]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-02-02 14:32:35]

Description was changed from

==========
[interpreter] Move temporary register allocator into own file.

Moves the temporary register allocator out of the bytecode array
builder into TemporaryRegisterAllocator class and adds unittests.
Particular must be taken around the translation window boundary
motivating the addition of tests.

Also adds a Clear() method to IdentityMap() which is called by
the destructor. This allows classes to hold an IdentityMap if
they are zone allocated. Classes must call Clear() before the zone
is re-cycled or face v8 heap corruption.

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

to

==========
[interpreter] Move temporary register allocator into own file.

Moves the temporary register allocator out of the bytecode array
builder into TemporaryRegisterAllocator class and adds unittests.
Particular must be taken around the translation window boundary
motivating the addition of tests.

Also adds a Clear() method to IdentityMap() which is called by
the destructor. This allows classes to hold an IdentityMap if
they are zone allocated. Classes must call Clear() before the zone
is re-cycled or face v8 heap corruption.

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

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

[commit-bot: I haz the power, 2016-02-02 14:32:36]

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