[Interpreter]: Add support for strict mode load / store ICs.

[Interpreter]: Add support for strict mode load / store ICs.

Adds support for strict mode load / store ICs and cleans up BinaryOp and
CompareOp to only trigger an UNIMPLEMENTED abort if called with STRONG
mode (which is the only language mode which has different compare/binary ops.

BUG=v8:4280
LOG=N

Committed: https://crrev.com/785516821de1d89320c26bdb3849173a1410abd2
Cr-Commit-Position: refs/heads/master@{#31134}

[rmcilroy, 2015-10-02 13:41:56]

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

[rmcilroy, 2015-10-02 13:41:56]

mvstanton: please take a look at IC calls.
oth: please take a look at interpreter changes.
mstarzinger: owner stamp for compiler/bytecode-graph-builder.cc change (review
welcome too).

[rmcilroy, 2015-10-02 14:13:06]

rmcilroy@chromium.org changed reviewers:
+ mstarzinger@chromium.org
- mstarzinger@chromium.o

[oth, 2015-10-02 15:17:44]

interpreter lgtm. optional nit.

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

https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-builder.cc:133: UNIMPLEMENTED();
Nitletino - other updates here take a more conservative approach, could change
the test to be is_sloppy||is_strict and UNIMPLEMENTED otherwise.

https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-builder.cc:143: if
(is_strong(strength(language_mode))) {
Nitletino -same here.

[Michael Starzinger, 2015-10-05 12:04:45]

LGTM, just nits.

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

https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-builder.cc:243: UNIMPLEMENTED();
nit: The strong case is UNIMPLEMENTED but the default case should actually be
UNREACHABLE, correct? Likewise below.

https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecodes.h
File src/interpreter/bytecodes.h (right):

https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecodes.h...
src/interpreter/bytecodes.h:54: V(KeyedLoadIC, OperandType::kReg8,
OperandType::kIdx8)                       \
nit: Weird ordering is that on purpose?

https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecodes.h...
src/interpreter/bytecodes.h:56: V(LoadIC, OperandType::kReg8,
OperandType::kIdx8)                            \
suggestion: What would you think about s/LoadIC/LoadICSloppy/ to make it
explicit here? Just a suggestion, totally up to you.

[rmcilroy, 2015-10-06 13:56:34]

Updated with review comments.

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

https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-builder.cc:133: UNIMPLEMENTED();
On 2015/10/02 15:17:44, oth wrote:
> Nitletino - other updates here take a more conservative approach, could change
> the test to be is_sloppy||is_strict and UNIMPLEMENTED otherwise.

I'd prefer not to do this since it is the language mode strength, not the
language mode itself, which makes the difference. However, changed the signature
to take a strength instead of language mode instead, how's that?

https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-builder.cc:143: if
(is_strong(strength(language_mode))) {
On 2015/10/02 15:17:44, oth wrote:
> Nitletino -same here.

Acknowledged.

https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-builder.cc:243: UNIMPLEMENTED();
On 2015/10/05 12:04:45, Michael Starzinger wrote:
> nit: The strong case is UNIMPLEMENTED but the default case should actually be
> UNREACHABLE, correct? Likewise below.

Done.

https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecodes.h
File src/interpreter/bytecodes.h (right):

https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecodes.h...
src/interpreter/bytecodes.h:54: V(KeyedLoadIC, OperandType::kReg8,
OperandType::kIdx8)                       \
On 2015/10/05 12:04:45, Michael Starzinger wrote:
> nit: Weird ordering is that on purpose?

Opps, mistake - fixed

https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecodes.h...
src/interpreter/bytecodes.h:56: V(LoadIC, OperandType::kReg8,
OperandType::kIdx8)                            \
On 2015/10/05 12:04:45, Michael Starzinger wrote:
> suggestion: What would you think about s/LoadIC/LoadICSloppy/ to make it
> explicit here? Just a suggestion, totally up to you.

Yeah I was thinking about that - I think it makes sense - done.

[oth, 2015-10-06 14:08:17]

On 2015/10/06 13:56:34, rmcilroy wrote:
> Updated with review comments.
> 
>
https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecode-ar...
> File src/interpreter/bytecode-array-builder.cc (right):
> 
>
https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecode-ar...
> src/interpreter/bytecode-array-builder.cc:133: UNIMPLEMENTED();
> On 2015/10/02 15:17:44, oth wrote:
> > Nitletino - other updates here take a more conservative approach, could
change
> > the test to be is_sloppy||is_strict and UNIMPLEMENTED otherwise.
> 
> I'd prefer not to do this since it is the language mode strength, not the
> language mode itself, which makes the difference. However, changed the
signature
> to take a strength instead of language mode instead, how's that?
> 
>
https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecode-ar...
> src/interpreter/bytecode-array-builder.cc:143: if
> (is_strong(strength(language_mode))) {
> On 2015/10/02 15:17:44, oth wrote:
> > Nitletino -same here.
> 
> Acknowledged.
> 
>
https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecode-ar...
> src/interpreter/bytecode-array-builder.cc:243: UNIMPLEMENTED();
> On 2015/10/05 12:04:45, Michael Starzinger wrote:
> > nit: The strong case is UNIMPLEMENTED but the default case should actually
be
> > UNREACHABLE, correct? Likewise below.
> 
> Done.
> 
> https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecodes.h
> File src/interpreter/bytecodes.h (right):
> 
>
https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecodes.h...
> src/interpreter/bytecodes.h:54: V(KeyedLoadIC, OperandType::kReg8,
> OperandType::kIdx8)                       \
> On 2015/10/05 12:04:45, Michael Starzinger wrote:
> > nit: Weird ordering is that on purpose?
> 
> Opps, mistake - fixed
> 
>
https://codereview.chromium.org/1385623002/diff/1/src/interpreter/bytecodes.h...
> src/interpreter/bytecodes.h:56: V(LoadIC, OperandType::kReg8,
> OperandType::kIdx8)                            \
> On 2015/10/05 12:04:45, Michael Starzinger wrote:
> > suggestion: What would you think about s/LoadIC/LoadICSloppy/ to make it
> > explicit here? Just a suggestion, totally up to you.
> 
> Yeah I was thinking about that - I think it makes sense - done.

Last set of changes are clearer, lgtm.

[rmcilroy, 2015-10-06 14:16:30]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2015-10-06 14:16:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1385623002/#ps20001
(title: "Review comments.")

[commit-bot: I haz the power, 2015-10-06 14:16:51]

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

[commit-bot: I haz the power, 2015-10-06 14:17:50]

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

[commit-bot: I haz the power, 2015-10-06 14:17:51]

Try jobs failed on following builders:
  v8_android_arm_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_mac_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/10404)

[rmcilroy, 2015-10-06 14:24:56]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2015-10-06 14:24:57]

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

[commit-bot: I haz the power, 2015-10-06 14:25:14]

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

[commit-bot: I haz the power, 2015-10-06 14:29:20]

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

[commit-bot: I haz the power, 2015-10-06 14:29:20]

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

[rmcilroy, 2015-10-07 07:31:10]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2015-10-07 07:31:10]

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

[commit-bot: I haz the power, 2015-10-07 07:31:22]

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

[commit-bot: I haz the power, 2015-10-07 07:54:13]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-07 07:54:38]

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

[mvstanton, 2015-10-07 09:11:45]

Sorry to be late on this one.
IC calls look okay, lgtm.