DBC: Implement InstanceOf.

DBC: Implement InstanceOf.

R=zra@google.com

Committed: https://github.com/dart-lang/sdk/commit/5548f4dd67d5b3264a1b5b291a5235c36e5383b6

[rmacnak, 2016-06-28 18:13:47]

rmacnak@google.com changed reviewers:
+ zra@google.com

[zra, 2016-06-28 19:28:05]

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/code_generat...
File runtime/vm/code_generator.cc (right):

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/code_generat...
runtime/vm/code_generator.cc:453: #if !defined(TARGET_ARCH_DBC)
It looks like you're passing the cache, so even though we're not checking it in
the simulator, these should still work shouldn't they?

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/constants_dbc.h
File runtime/vm/constants_dbc.h (right):

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/constants_db...
runtime/vm/constants_dbc.h:477: V(LogicalNot,                      0, ___, ___,
___) \
Already have BooleanNegate{TOS}.

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/constants_db...
runtime/vm/constants_dbc.h:495: V(BooleanNegateTOS,                0, ___, ___,
___) \
Here.

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_dbc.cc (right):

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/intermediate...
runtime/vm/intermediate_language_dbc.cc:202: if (negate_result()) {
I think we should fold this into the InstanceOf instruction, i.e.
InstanceOf(negate_result() ? 1 : 0).

[zra, 2016-06-28 19:30:36]

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/simulator_db...
File runtime/vm/simulator_dbc.cc (right):

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/simulator_db...
runtime/vm/simulator_dbc.cc:1858: RawInstance* instance =
static_cast<RawInstance*>(SP[-3]);
If you use the RAW_CAST macro, we'll also get some good ASSERTs for these in
Debug builds.

[rmacnak, 2016-06-28 21:21:39]

Description was changed from

==========
DBC: Implement InstanceOf as a runtime call.
==========

to

==========
DBC: Implement InstanceOf.
==========

[rmacnak, 2016-06-28 21:21:50]

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/code_generat...
File runtime/vm/code_generator.cc (right):

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/code_generat...
runtime/vm/code_generator.cc:453: #if !defined(TARGET_ARCH_DBC)
On 2016/06/28 19:28:04, zra wrote:
> It looks like you're passing the cache, so even though we're not checking it
in
> the simulator, these should still work shouldn't they?

The update code is unhappy if it inserts duplicates. Added the code to actually
check the cache.

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/constants_dbc.h
File runtime/vm/constants_dbc.h (right):

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/constants_db...
runtime/vm/constants_dbc.h:477: V(LogicalNot,                      0, ___, ___,
___) \
On 2016/06/28 19:28:05, zra wrote:
> Already have BooleanNegate{TOS}.

Ahh

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_dbc.cc (right):

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/intermediate...
runtime/vm/intermediate_language_dbc.cc:202: if (negate_result()) {
On 2016/06/28 19:28:05, zra wrote:
> I think we should fold this into the InstanceOf instruction, i.e.
> InstanceOf(negate_result() ? 1 : 0).

Done.

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/simulator_db...
File runtime/vm/simulator_dbc.cc (right):

https://codereview.chromium.org/2109743002/diff/20001/runtime/vm/simulator_db...
runtime/vm/simulator_dbc.cc:1858: RawInstance* instance =
static_cast<RawInstance*>(SP[-3]);
On 2016/06/28 19:30:35, zra wrote:
> If you use the RAW_CAST macro, we'll also get some good ASSERTs for these in
> Debug builds.

RAW_CAST is too strict here. type_args may be null, type may be Type or
TypeParameter or FunctionType, etc.

[zra, 2016-06-28 21:37:47]

lgtm with small fixes.

https://codereview.chromium.org/2109743002/diff/40001/runtime/vm/constants_dbc.h
File runtime/vm/constants_dbc.h (right):

https://codereview.chromium.org/2109743002/diff/40001/runtime/vm/constants_db...
runtime/vm/constants_dbc.h:346: //    (A = 1) a subtype of SP[-1] using
SubtypeTestCache SP[0].
With result at top of stack.

https://codereview.chromium.org/2109743002/diff/40001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_dbc.cc (right):

https://codereview.chromium.org/2109743002/diff/40001/runtime/vm/intermediate...
runtime/vm/intermediate_language_dbc.cc:181:
DEFINE_MAKE_LOCATION_SUMMARY(InstanceOf, 2,
This can just be

EMIT_NATIVE_CODE(InstanceOf, 2, Location::SameAsFirstInput(),
LocationSummary::kCall) {
  ...
}

[rmacnak, 2016-06-28 22:22:55]

https://codereview.chromium.org/2109743002/diff/40001/runtime/vm/constants_dbc.h
File runtime/vm/constants_dbc.h (right):

https://codereview.chromium.org/2109743002/diff/40001/runtime/vm/constants_db...
runtime/vm/constants_dbc.h:346: //    (A = 1) a subtype of SP[-1] using
SubtypeTestCache SP[0].
On 2016/06/28 21:37:47, zra wrote:
> With result at top of stack.

Done.

https://codereview.chromium.org/2109743002/diff/40001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_dbc.cc (right):

https://codereview.chromium.org/2109743002/diff/40001/runtime/vm/intermediate...
runtime/vm/intermediate_language_dbc.cc:181:
DEFINE_MAKE_LOCATION_SUMMARY(InstanceOf, 2,
On 2016/06/28 21:37:47, zra wrote:
> This can just be
> 
> EMIT_NATIVE_CODE(InstanceOf, 2, Location::SameAsFirstInput(),
> LocationSummary::kCall) {
>   ...
> }

Done.

[rmacnak, 2016-06-28 22:45:31]

Description was changed from

==========
DBC: Implement InstanceOf.
==========

to

==========
DBC: Implement InstanceOf.

R=zra@google.com

Committed:
https://github.com/dart-lang/sdk/commit/5548f4dd67d5b3264a1b5b291a5235c36e5383b6
==========

[rmacnak, 2016-06-28 22:45:32]

Committed patchset #4 (id:60001) manually as
5548f4dd67d5b3264a1b5b291a5235c36e5383b6 (presubmit successful).