[Interpreter] Make ForInPrepare take a kRegTriple8 and ForInNext take kRegPair8 for cache state

[Interpreter] Make ForInPrepare take a kRegTriple8 and ForInNext take kRegPair8 for cache state

Make ForInPrepare take a kRegTriple8 operand and ForInNext take kRegPair8
operand for cache state. This is to ensure that the cache state output of
ForInPrepare is in consecutive registers to allow us to deopt the
ForInPrepare node from TF->Ignition (to be done in a followup CL).

BUG=v8:4280
LOG=N

Committed: https://crrev.com/1ea0b91a833197d5e43f147129fa7aba5f659217
Cr-Commit-Position: refs/heads/master@{#33357}

[rmcilroy, 2016-01-13 14:42:14]

Description was changed from

==========
[Interpreter] Make ForInPrepare take a kRegTriple8 operand and ForInNext take
kRegPair8 operand for cache state.

Make ForInPrepare take a kRegTriple8 operand and ForInNext take kRegPair8
operand for cache state. This is to ensure that the cache state output of
ForInPrepare is in consecutive registers to allow us to deopt the
ForInPrepare node from TF->Ignition (to be done in a followup CL).

BUG=v8:4280
LOG=N
==========

to

==========
[Interpreter] Make ForInPrepare take a kRegTriple8 and ForInNext take kRegPair8
for cache state

Make ForInPrepare take a kRegTriple8 operand and ForInNext take kRegPair8
operand for cache state. This is to ensure that the cache state output of
ForInPrepare is in consecutive registers to allow us to deopt the
ForInPrepare node from TF->Ignition (to be done in a followup CL).

BUG=v8:4280
LOG=N
==========

[rmcilroy, 2016-01-13 14:43:26]

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

[commit-bot: I haz the power, 2016-01-13 14:43:30]

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

[rmcilroy, 2016-01-13 14:43:56]

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

[rmcilroy, 2016-01-13 14:43:56]

Benedikt, please look at compiler changes.

Orion, please look at overall CL.

Thanks!

[Benedikt Meurer, 2016-01-13 14:49:46]

LGTM on compiler.

[commit-bot: I haz the power, 2016-01-13 15:02:49]

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

[commit-bot: I haz the power, 2016-01-13 15:02:49]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...)
  v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mipsel_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)

[oth, 2016-01-13 15:28:58]

lgtm. A few naming suggestions for readability, but fine.

https://codereview.chromium.org/1584813002/diff/1/src/interpreter/bytecode-ar...
File src/interpreter/bytecode-array-builder.cc (right):

https://codereview.chromium.org/1584813002/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-builder.cc:977: BytecodeArrayBuilder&
BytecodeArrayBuilder::ForInPrepare(Register first_reg) {
Nit, argument name generic, s/first_reg/cache_info_triple/

https://codereview.chromium.org/1584813002/diff/1/src/interpreter/bytecode-ar...
File src/interpreter/bytecode-array-builder.h (right):

https://codereview.chromium.org/1584813002/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-builder.h:227: Register cache_register_pair);
The name of the third argument differs in the .cc file.

As the the pair is cache_type and cache_array, it might be better to have the
name hint at it, e.g. cache_type_array_pair.

https://codereview.chromium.org/1584813002/diff/1/src/interpreter/interpreter.cc
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/1584813002/diff/1/src/interpreter/interpreter...
src/interpreter/interpreter.cc:1694: // ForInPrepare <first_register>
cache_info_triple?

[rmcilroy, 2016-01-18 09:40:49]

https://codereview.chromium.org/1584813002/diff/1/src/interpreter/bytecode-ar...
File src/interpreter/bytecode-array-builder.cc (right):

https://codereview.chromium.org/1584813002/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-builder.cc:977: BytecodeArrayBuilder&
BytecodeArrayBuilder::ForInPrepare(Register first_reg) {
On 2016/01/13 15:28:58, oth wrote:
> Nit, argument name generic, s/first_reg/cache_info_triple/ 

Done.

https://codereview.chromium.org/1584813002/diff/1/src/interpreter/bytecode-ar...
File src/interpreter/bytecode-array-builder.h (right):

https://codereview.chromium.org/1584813002/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-builder.h:227: Register cache_register_pair);
On 2016/01/13 15:28:58, oth wrote:
> The name of the third argument differs in the .cc file.
> 
> As the the pair is cache_type and cache_array, it might be better to have the
> name hint at it, e.g. cache_type_array_pair.

Good idea, done.

https://codereview.chromium.org/1584813002/diff/1/src/interpreter/interpreter.cc
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/1584813002/diff/1/src/interpreter/interpreter...
src/interpreter/interpreter.cc:1694: // ForInPrepare <first_register>
On 2016/01/13 15:28:58, oth wrote:
> cache_info_triple?

Done.

[rmcilroy, 2016-01-18 09:40:53]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2016-01-18 09:40:53]

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

[commit-bot: I haz the power, 2016-01-18 09:41:03]

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

[commit-bot: I haz the power, 2016-01-18 09:43:17]

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

[commit-bot: I haz the power, 2016-01-18 09:43:17]

Try jobs failed on following builders:
  v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...)

[rmcilroy, 2016-01-18 11:58:48]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2016-01-18 11:58:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, oth@chromium.org
Link to the patchset: https://codereview.chromium.org/1584813002/#ps60001
(title: "Fix release")

[commit-bot: I haz the power, 2016-01-18 11:58:58]

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

[commit-bot: I haz the power, 2016-01-18 12:39:25]

Description was changed from

==========
[Interpreter] Make ForInPrepare take a kRegTriple8 and ForInNext take kRegPair8
for cache state

Make ForInPrepare take a kRegTriple8 operand and ForInNext take kRegPair8
operand for cache state. This is to ensure that the cache state output of
ForInPrepare is in consecutive registers to allow us to deopt the
ForInPrepare node from TF->Ignition (to be done in a followup CL).

BUG=v8:4280
LOG=N
==========

to

==========
[Interpreter] Make ForInPrepare take a kRegTriple8 and ForInNext take kRegPair8
for cache state

Make ForInPrepare take a kRegTriple8 operand and ForInNext take kRegPair8
operand for cache state. This is to ensure that the cache state output of
ForInPrepare is in consecutive registers to allow us to deopt the
ForInPrepare node from TF->Ignition (to be done in a followup CL).

BUG=v8:4280
LOG=N
==========

[commit-bot: I haz the power, 2016-01-18 12:39:25]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-01-18 12:40:27]

Description was changed from

==========
[Interpreter] Make ForInPrepare take a kRegTriple8 and ForInNext take kRegPair8
for cache state

Make ForInPrepare take a kRegTriple8 operand and ForInNext take kRegPair8
operand for cache state. This is to ensure that the cache state output of
ForInPrepare is in consecutive registers to allow us to deopt the
ForInPrepare node from TF->Ignition (to be done in a followup CL).

BUG=v8:4280
LOG=N
==========

to

==========
[Interpreter] Make ForInPrepare take a kRegTriple8 and ForInNext take kRegPair8
for cache state

Make ForInPrepare take a kRegTriple8 operand and ForInNext take kRegPair8
operand for cache state. This is to ensure that the cache state output of
ForInPrepare is in consecutive registers to allow us to deopt the
ForInPrepare node from TF->Ignition (to be done in a followup CL).

BUG=v8:4280
LOG=N

Committed: https://crrev.com/1ea0b91a833197d5e43f147129fa7aba5f659217
Cr-Commit-Position: refs/heads/master@{#33357}
==========

[commit-bot: I haz the power, 2016-01-18 12:40:28]

Patchset 4 (id:??) landed as
https://crrev.com/1ea0b91a833197d5e43f147129fa7aba5f659217
Cr-Commit-Position: refs/heads/master@{#33357}