[Interpreter] Optimize equality check with null/undefined with a check on the map.

[Interpreter] Optimize equality check with null/undefined with a check on the map.

Equality with null/undefined is equivalent to a check on the undetectable bit
on the map of the object. This would be more efficient than performing the entire
comparison operation.

This cl introduces:
1. A new bytecode called TestUndetectable that checks if the object is null/undefined.
2. Updates peeophole optimizer to emit TestUndetectable when a LdaNull/Undefined
precedes equality check.
4. TestUndetectable is transformed to ObjectIsUndetectable operator when building
turbofan graph.

BUG=v8:4280
Committed: https://crrev.com/9119d16904e9062d319427029c2c74270776e68e
Cr-Commit-Position: refs/heads/master@{#41514}

[mythria, 2016-12-02 15:19:06]

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

[commit-bot: I haz the power, 2016-12-02 15:19:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-02 15:38:43]

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

[commit-bot: I haz the power, 2016-12-02 15:38:43]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)

[mythria, 2016-12-02 16:36:51]

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

[commit-bot: I haz the power, 2016-12-02 16:36:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-02 17:03:07]

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

[commit-bot: I haz the power, 2016-12-02 17:03:08]

Dry run: This issue passed the CQ dry run.

[mythria, 2016-12-05 09:58:36]

Description was changed from

==========
[Interpreter] Simplify equality check with null/undefined by a check on the map.

Equality with null/undefined is equivalent to a check on the undetectable bit
on the map of the object. This would be much simpler than performing the entire
comparison operation.

This cl introduces:
1. A new bytecode called TestUndetectable that checks if the object is
null/undefined.
2. Updates peeophole optimizer to emit TestUndetectable when a LdaNull/Undefined
precedes
   equality check.
3. Introduce a new JSOperator, JSIsUndetectable to translate TestUndetectable
bytecode
   to turbofan graph.
4. JSIsUndetectable is simplified to ObjectIsUndetectable in the typed lowering
phase.

BUG=v8:4280
==========

to

==========
[Interpreter] Optimize equality check with null/undefined with a check on the
map.

Equality with null/undefined is equivalent to a check on the undetectable bit
on the map of the object. This would be much simpler than performing the entire
comparison operation.

This cl introduces:
1. A new bytecode called TestUndetectable that checks if the object is
null/undefined.
2. Updates peeophole optimizer to emit TestUndetectable when a LdaNull/Undefined
precedes
   equality check.
3. Introduce a new JSOperator, JSIsUndetectable to translate TestUndetectable
bytecode
   to turbofan graph.
4. JSIsUndetectable is simplified to ObjectIsUndetectable in the typed lowering
phase.

BUG=v8:4280
==========

[mythria, 2016-12-05 09:58:56]

Description was changed from

==========
[Interpreter] Optimize equality check with null/undefined with a check on the
map.

Equality with null/undefined is equivalent to a check on the undetectable bit
on the map of the object. This would be much simpler than performing the entire
comparison operation.

This cl introduces:
1. A new bytecode called TestUndetectable that checks if the object is
null/undefined.
2. Updates peeophole optimizer to emit TestUndetectable when a LdaNull/Undefined
precedes
   equality check.
3. Introduce a new JSOperator, JSIsUndetectable to translate TestUndetectable
bytecode
   to turbofan graph.
4. JSIsUndetectable is simplified to ObjectIsUndetectable in the typed lowering
phase.

BUG=v8:4280
==========

to

==========
[Interpreter] Optimize equality check with null/undefined with a check on the
map.

Equality with null/undefined is equivalent to a check on the undetectable bit
on the map of the object. This would be more efficient than performing the
entire
comparison operation.

This cl introduces:
1. A new bytecode called TestUndetectable that checks if the object is
null/undefined.
2. Updates peeophole optimizer to emit TestUndetectable when a LdaNull/Undefined
precedes
   equality check.
3. Introduce a new JSOperator, JSIsUndetectable to translate TestUndetectable
bytecode
   to turbofan graph.
4. JSIsUndetectable is simplified to ObjectIsUndetectable in the typed lowering
phase.

BUG=v8:4280
==========

[mythria, 2016-12-05 10:05:42]

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

[mythria, 2016-12-05 10:05:42]

This cl simplified checks with null/undefined by a check on the undetectable bit
on the map of the object, similar to what FCG does.

PTAL.

Thanks,
Mythri

https://codereview.chromium.org/2547043002/diff/20001/src/compiler/js-operato...
File src/compiler/js-operator.cc (right):

https://codereview.chromium.org/2547043002/diff/20001/src/compiler/js-operato...
src/compiler/js-operator.cc:606: IrOpcode::kJSIsUndetectable, Operator::kPure, 
// opcode
If my understanding is correct, this operation does not throw, does not deopt
and also does not impact effect edges. This operation translates to
ObjectIsUndetectable. 

Also do we need to cache this operator?

[rmcilroy, 2016-12-05 10:57:19]

LGTM once comments are addressed.

One question - do we need a separate js-operator for this, or can we just use
the simple operator?

https://codereview.chromium.org/2547043002/diff/20001/src/compiler/js-generic...
File src/compiler/js-generic-lowering.cc (right):

https://codereview.chromium.org/2547043002/diff/20001/src/compiler/js-generic...
src/compiler/js-generic-lowering.cc:137: UNREACHABLE();  // Should be replaced
by ObjectIsUndetectable.
Nit - in typed lowering.

https://codereview.chromium.org/2547043002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-peephole-optimizer.cc (right):

https://codereview.chromium.org/2547043002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-peephole-optimizer.cc:146:
current->set_bytecode(new_bytecode, current->operand(0));
I don't think you need to pass the new_bytecode  (or have it as action data on
the peephole table) since it should always be TestUndetectable, right? Could you
change to remove the action data?

https://codereview.chromium.org/2547043002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-peephole-optimizer.cc:270: if
(!node->source_info().is_valid() || !last()->source_info().is_valid()) {
Do we need to do this check? LdaUndefined/null can never through, so I don't
think the first source position is ever observable.

https://codereview.chromium.org/2547043002/diff/20001/src/interpreter/mkpeeph...
File src/interpreter/mkpeephole.cc (right):

https://codereview.chromium.org/2547043002/diff/20001/src/interpreter/mkpeeph...
src/interpreter/mkpeephole.cc:204: Bytecode::kTestUndetectable};
Could we also fuse NotEquals too (with another bytecode for
TestNotUndetectable)? Would that be worth it?

https://codereview.chromium.org/2547043002/diff/20001/test/cctest/interpreter...
File test/cctest/interpreter/bytecode_expectations/Generators.golden (right):

https://codereview.chromium.org/2547043002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/bytecode_expectations/Generators.golden:479:
B(JumpIfToBooleanFalse), U8(4),
Could you add an explicit BytecodeGeneratorTest for this logic please?

https://codereview.chromium.org/2547043002/diff/20001/test/cctest/test-serial...
File test/cctest/test-serialize.cc (right):

https://codereview.chromium.org/2547043002/diff/20001/test/cctest/test-serial...
test/cctest/test-serialize.cc:1078: STATIC_CHAR_VECTOR("} j=7; j"), 1050);
Why this change?

[Benedikt Meurer, 2016-12-05 11:29:12]

LGTM % question: Wouldn't it make sense to have a JumpIfIsUndetectable
instruction then?

[rmcilroy, 2016-12-05 14:14:35]

On 2016/12/05 11:29:12, Benedikt Meurer wrote:
> LGTM % question: Wouldn't it make sense to have a JumpIfIsUndetectable
> instruction then?

Sounds reasonable. Let's do this as a follow-up CL though so any perf impact is
visible on the bots.

[mythria, 2016-12-05 14:25:40]

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

[commit-bot: I haz the power, 2016-12-05 14:25:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-05 14:51:49]

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

[commit-bot: I haz the power, 2016-12-05 14:51:50]

Dry run: This issue passed the CQ dry run.

[mythria, 2016-12-05 14:53:11]

Thanks Benedikt and Ross. 

Benedikt, That is a good suggestion. I will do the JumpIfUndetectable
optimization in a follow up cl.

Ross, I fixed most of the comments. I think we don't need the JSOperator for
Undetectable, we could directly reduce it to the simplified operator. I will fix
it in the next patch. I forgot to do it in this one.

Also, I haven't added TestNotUndetectable. FCG does not seem to do this. I don't
see any reason why we shouldn't do this though. Should I do this in a followup
cl?

https://codereview.chromium.org/2547043002/diff/20001/src/compiler/js-generic...
File src/compiler/js-generic-lowering.cc (right):

https://codereview.chromium.org/2547043002/diff/20001/src/compiler/js-generic...
src/compiler/js-generic-lowering.cc:137: UNREACHABLE();  // Should be replaced
by ObjectIsUndetectable.
On 2016/12/05 10:57:19, rmcilroy wrote:
> Nit - in typed lowering.

Done.

https://codereview.chromium.org/2547043002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-peephole-optimizer.cc (right):

https://codereview.chromium.org/2547043002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-peephole-optimizer.cc:146:
current->set_bytecode(new_bytecode, current->operand(0));
On 2016/12/05 10:57:19, rmcilroy wrote:
> I don't think you need to pass the new_bytecode  (or have it as action data on
> the peephole table) since it should always be TestUndetectable, right? Could
you
> change to remove the action data? 

Yes, we don't need action data. I did not think about that.

Done.

https://codereview.chromium.org/2547043002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-peephole-optimizer.cc:270: if
(!node->source_info().is_valid() || !last()->source_info().is_valid()) {
On 2016/12/05 10:57:19, rmcilroy wrote:
> Do we need to do this check? LdaUndefined/null can never through, so I don't
> think the first source position is ever observable.

I don't think we need, because if my understanding is correct the last source
info can only be ExpressionPosition and LdaNull/Undefined cannot throw. I also
think we don't need them for BinaryOpWithSmi or BinaryOpWithZero optimizations.
Since LdaSmi/LdaZero also cannot throw. I removed the check here. If you think
we also don't need it with BinaryOps, I will also remove them.

https://codereview.chromium.org/2547043002/diff/20001/src/interpreter/mkpeeph...
File src/interpreter/mkpeephole.cc (right):

https://codereview.chromium.org/2547043002/diff/20001/src/interpreter/mkpeeph...
src/interpreter/mkpeephole.cc:204: Bytecode::kTestUndetectable};
On 2016/12/05 10:57:19, rmcilroy wrote:
> Could we also fuse NotEquals too (with another bytecode for
> TestNotUndetectable)? Would that be worth it? 

Full-codegen does not do this, so I wasn't sure we need this. Should I do it in
a follow-up cl?

https://codereview.chromium.org/2547043002/diff/20001/test/cctest/interpreter...
File test/cctest/interpreter/bytecode_expectations/Generators.golden (right):

https://codereview.chromium.org/2547043002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/bytecode_expectations/Generators.golden:479:
B(JumpIfToBooleanFalse), U8(4),
On 2016/12/05 10:57:19, rmcilroy wrote:
> Could you add an explicit BytecodeGeneratorTest for this logic please?

Done.

https://codereview.chromium.org/2547043002/diff/20001/test/cctest/test-serial...
File test/cctest/test-serialize.cc (right):

https://codereview.chromium.org/2547043002/diff/20001/test/cctest/test-serial...
test/cctest/test-serialize.cc:1078: STATIC_CHAR_VECTOR("} j=7; j"), 1050);
On 2016/12/05 10:57:19, rmcilroy wrote:
> Why this change?

This test is to check if the code objects goes to large object space. With this
new optimization with 1000 repetitions it does not go into large object space,
so I had to increase the size to push it to large object space.

[mythria, 2016-12-05 15:21:28]

Description was changed from

==========
[Interpreter] Optimize equality check with null/undefined with a check on the
map.

Equality with null/undefined is equivalent to a check on the undetectable bit
on the map of the object. This would be more efficient than performing the
entire
comparison operation.

This cl introduces:
1. A new bytecode called TestUndetectable that checks if the object is
null/undefined.
2. Updates peeophole optimizer to emit TestUndetectable when a LdaNull/Undefined
precedes
   equality check.
3. Introduce a new JSOperator, JSIsUndetectable to translate TestUndetectable
bytecode
   to turbofan graph.
4. JSIsUndetectable is simplified to ObjectIsUndetectable in the typed lowering
phase.

BUG=v8:4280
==========

to

==========
[Interpreter] Optimize equality check with null/undefined with a check on the
map.

Equality with null/undefined is equivalent to a check on the undetectable bit
on the map of the object. This would be more efficient than performing the
entire
comparison operation.

This cl introduces:
1. A new bytecode called TestUndetectable that checks if the object is
null/undefined.
2. Updates peeophole optimizer to emit TestUndetectable when a LdaNull/Undefined
precedes
   equality check.
4. TestUndetectable is transformed to ObjectIsUndetectable operator when
building turbofan graph.

BUG=v8:4280
==========

[mythria, 2016-12-05 15:21:59]

Description was changed from

==========
[Interpreter] Optimize equality check with null/undefined with a check on the
map.

Equality with null/undefined is equivalent to a check on the undetectable bit
on the map of the object. This would be more efficient than performing the
entire
comparison operation.

This cl introduces:
1. A new bytecode called TestUndetectable that checks if the object is
null/undefined.
2. Updates peeophole optimizer to emit TestUndetectable when a LdaNull/Undefined
precedes
   equality check.
4. TestUndetectable is transformed to ObjectIsUndetectable operator when
building turbofan graph.

BUG=v8:4280
==========

to

==========
[Interpreter] Optimize equality check with null/undefined with a check on the
map.

Equality with null/undefined is equivalent to a check on the undetectable bit
on the map of the object. This would be more efficient than performing the
entire
comparison operation.

This cl introduces:
1. A new bytecode called TestUndetectable that checks if the object is
null/undefined.
2. Updates peeophole optimizer to emit TestUndetectable when a LdaNull/Undefined
precedes equality check.
4. TestUndetectable is transformed to ObjectIsUndetectable operator when
building
turbofan graph.

BUG=v8:4280
==========

[mythria, 2016-12-05 15:22:21]

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

[commit-bot: I haz the power, 2016-12-05 15:22:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mythria, 2016-12-05 15:25:30]

When building turbofan graph for TestUndetectable, now I use the simplified
operator ObjectIsUndetectable directly. So we need not introduce a new
JSOperator now. The code in the compiler is much simpler now, with only changes
in bytecode graph builder. PTAL.

Thanks,
Mythri.

[commit-bot: I haz the power, 2016-12-05 15:48:52]

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

[commit-bot: I haz the power, 2016-12-05 15:48:53]

Dry run: This issue passed the CQ dry run.

[mythria, 2016-12-05 17:20:34]

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

[commit-bot: I haz the power, 2016-12-05 17:20:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-05 17:46:52]

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

[commit-bot: I haz the power, 2016-12-05 17:46:53]

Dry run: This issue passed the CQ dry run.

[mythria, 2016-12-05 17:48:12]

I am sorry for so many mails and patches. I am somehow seem to be missing
everything today. PTAL at the latest patch. I migrated any important comments
from my earlier patches. PTAL. This cl does not seem to improve performance
much. 

Thanks,
Mythri

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

https://codereview.chromium.org/2547043002/diff/80001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1836: if
(variable->raw_name()->IsOneByteEqualTo("undefined")) {
I took this logic from here:
https://cs.chromium.org/chromium/src/v8/src/ast/ast.cc?sq=package:chromium&rc...

I think it is safe to do this optimization right? or Am I missing something?

https://codereview.chromium.org/2547043002/diff/80001/src/interpreter/mkpeeph...
File src/interpreter/mkpeephole.cc (right):

https://codereview.chromium.org/2547043002/diff/80001/src/interpreter/mkpeeph...
src/interpreter/mkpeephole.cc:201: if (current == Bytecode::kTestEqual) {
Ast Converts b != null to b == null. So we don't need another bytecode for
TestNotUndetectable.

https://codereview.chromium.org/2547043002/diff/80001/test/cctest/test-serial...
File test/cctest/test-serialize.cc (right):

https://codereview.chromium.org/2547043002/diff/80001/test/cctest/test-serial...
test/cctest/test-serialize.cc:1078: STATIC_CHAR_VECTOR("} j=7; j"), 1050);
This test checks if large code objects are allocated in large object space.
After this optimization, I had to increase the size for it to be allocated in
the large object space.

[Benedikt Meurer, 2016-12-05 18:30:12]

Ack. LGTM still.

[rmcilroy, 2016-12-05 19:08:09]

LGTM, thanks.

https://codereview.chromium.org/2547043002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-peephole-optimizer.cc (right):

https://codereview.chromium.org/2547043002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-peephole-optimizer.cc:270: if
(!node->source_info().is_valid() || !last()->source_info().is_valid()) {
On 2016/12/05 14:53:11, mythria wrote:
> On 2016/12/05 10:57:19, rmcilroy wrote:
> > Do we need to do this check? LdaUndefined/null can never through, so I don't
> > think the first source position is ever observable.
> 
> I don't think we need, because if my understanding is correct the last source
> info can only be ExpressionPosition and LdaNull/Undefined cannot throw. I also
> think we don't need them for BinaryOpWithSmi or BinaryOpWithZero
optimizations.
> Since LdaSmi/LdaZero also cannot throw. I removed the check here. If you think
> we also don't need it with BinaryOps, I will also remove them.

Yeah we could think about removing for BinaryOpWithSmi or BinaryOpWithZero.
Please do this in another CL if you want do do it.

https://codereview.chromium.org/2547043002/diff/20001/test/cctest/test-serial...
File test/cctest/test-serialize.cc (right):

https://codereview.chromium.org/2547043002/diff/20001/test/cctest/test-serial...
test/cctest/test-serialize.cc:1078: STATIC_CHAR_VECTOR("} j=7; j"), 1050);
On 2016/12/05 14:53:11, mythria wrote:
> On 2016/12/05 10:57:19, rmcilroy wrote:
> > Why this change?
> 
> This test is to check if the code objects goes to large object space. With
this
> new optimization with 1000 repetitions it does not go into large object space,
> so I had to increase the size to push it to large object space.

Acknowledged.

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

https://codereview.chromium.org/2547043002/diff/80001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1836: if
(variable->raw_name()->IsOneByteEqualTo("undefined")) {
On 2016/12/05 17:48:12, mythria wrote:
> I took this logic from here:
>
https://cs.chromium.org/chromium/src/v8/src/ast/ast.cc?sq=package:chromium&rc...
> 
> I think it is safe to do this optimization right? or Am I missing something?

It's possibly slow to check IsOneByteEqualTo each time here. Could you instead
cache the AstRawString value of  ast_value_factory->undefined_string() in the
constructor and do a straight pointer comparison with that instead here (I think
that should catch all the occurrences that this would).

https://codereview.chromium.org/2547043002/diff/80001/src/interpreter/bytecod...
File src/interpreter/bytecode-peephole-optimizer.cc (right):

https://codereview.chromium.org/2547043002/diff/80001/src/interpreter/bytecod...
src/interpreter/bytecode-peephole-optimizer.cc:144: (last->bytecode() ==
Bytecode::kLdaUndefined));
Do you want to DCHECK that this current bytecode is a TestEqual here?

https://codereview.chromium.org/2547043002/diff/80001/src/interpreter/mkpeeph...
File src/interpreter/mkpeephole.cc (right):

https://codereview.chromium.org/2547043002/diff/80001/src/interpreter/mkpeeph...
src/interpreter/mkpeephole.cc:201: if (current == Bytecode::kTestEqual) {
On 2016/12/05 17:48:12, mythria wrote:
> Ast Converts b != null to b == null. So we don't need another bytecode for
> TestNotUndetectable.

Great thanks for checking.

https://codereview.chromium.org/2547043002/diff/80001/src/interpreter/mkpeeph...
src/interpreter/mkpeephole.cc:204: Bytecode::kTestUndetectable};
Still passing the bytecode as action data here. I think ti should just be
Bytecode::kIllegal.

https://codereview.chromium.org/2547043002/diff/80001/test/cctest/test-serial...
File test/cctest/test-serialize.cc (right):

https://codereview.chromium.org/2547043002/diff/80001/test/cctest/test-serial...
test/cctest/test-serialize.cc:1078: STATIC_CHAR_VECTOR("} j=7; j"), 1050);
On 2016/12/05 17:48:12, mythria wrote:
> This test checks if large code objects are allocated in large object space.
> After this optimization, I had to increase the size for it to be allocated in
> the large object space. 

Acknowledged.

[mythria, 2016-12-06 10:11:07]

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

[commit-bot: I haz the power, 2016-12-06 10:11:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mythria, 2016-12-06 10:13:27]

Thanks for your reviews. 

Ross, Fixed all of them.

Thanks,
Mythri

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

https://codereview.chromium.org/2547043002/diff/80001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1836: if
(variable->raw_name()->IsOneByteEqualTo("undefined")) {
On 2016/12/05 19:08:09, rmcilroy wrote:
> On 2016/12/05 17:48:12, mythria wrote:
> > I took this logic from here:
> >
>
https://cs.chromium.org/chromium/src/v8/src/ast/ast.cc?sq=package:chromium&rc...
> > 
> > I think it is safe to do this optimization right? or Am I missing something?
> 
> It's possibly slow to check IsOneByteEqualTo each time here. Could you instead
> cache the AstRawString value of  ast_value_factory->undefined_string() in the
> constructor and do a straight pointer comparison with that instead here (I
think
> that should catch all the occurrences that this would).

Nice one. Done.

https://codereview.chromium.org/2547043002/diff/80001/src/interpreter/bytecod...
File src/interpreter/bytecode-peephole-optimizer.cc (right):

https://codereview.chromium.org/2547043002/diff/80001/src/interpreter/bytecod...
src/interpreter/bytecode-peephole-optimizer.cc:144: (last->bytecode() ==
Bytecode::kLdaUndefined));
On 2016/12/05 19:08:09, rmcilroy wrote:
> Do you want to DCHECK that this current bytecode is a TestEqual here?

Done.

https://codereview.chromium.org/2547043002/diff/80001/src/interpreter/mkpeeph...
File src/interpreter/mkpeephole.cc (right):

https://codereview.chromium.org/2547043002/diff/80001/src/interpreter/mkpeeph...
src/interpreter/mkpeephole.cc:204: Bytecode::kTestUndetectable};
On 2016/12/05 19:08:09, rmcilroy wrote:
> Still passing the bytecode as action data here. I think ti should just be
> Bytecode::kIllegal.

Done.

[commit-bot: I haz the power, 2016-12-06 10:36:59]

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

[commit-bot: I haz the power, 2016-12-06 10:37:00]

Dry run: This issue passed the CQ dry run.

[mythria, 2016-12-06 11:29:57]

The CQ bit was checked by mythria@chromium.org

[mythria, 2016-12-06 11:29:57]

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

[commit-bot: I haz the power, 2016-12-06 11:30:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-06 11:32:10]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1481023797134920,
"parent_rev": "e418de80bcc067b73de6306c60b96b61923c2ade", "commit_rev":
"d27176b19c77abed6bb15c2c5e001429fc8e95de"}

[commit-bot: I haz the power, 2016-12-06 11:32:16]

Description was changed from

==========
[Interpreter] Optimize equality check with null/undefined with a check on the
map.

Equality with null/undefined is equivalent to a check on the undetectable bit
on the map of the object. This would be more efficient than performing the
entire
comparison operation.

This cl introduces:
1. A new bytecode called TestUndetectable that checks if the object is
null/undefined.
2. Updates peeophole optimizer to emit TestUndetectable when a LdaNull/Undefined
precedes equality check.
4. TestUndetectable is transformed to ObjectIsUndetectable operator when
building
turbofan graph.

BUG=v8:4280
==========

to

==========
[Interpreter] Optimize equality check with null/undefined with a check on the
map.

Equality with null/undefined is equivalent to a check on the undetectable bit
on the map of the object. This would be more efficient than performing the
entire
comparison operation.

This cl introduces:
1. A new bytecode called TestUndetectable that checks if the object is
null/undefined.
2. Updates peeophole optimizer to emit TestUndetectable when a LdaNull/Undefined
precedes equality check.
4. TestUndetectable is transformed to ObjectIsUndetectable operator when
building
turbofan graph.

BUG=v8:4280
==========

[commit-bot: I haz the power, 2016-12-06 11:32:17]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-12-06 11:32:43]

Description was changed from

==========
[Interpreter] Optimize equality check with null/undefined with a check on the
map.

Equality with null/undefined is equivalent to a check on the undetectable bit
on the map of the object. This would be more efficient than performing the
entire
comparison operation.

This cl introduces:
1. A new bytecode called TestUndetectable that checks if the object is
null/undefined.
2. Updates peeophole optimizer to emit TestUndetectable when a LdaNull/Undefined
precedes equality check.
4. TestUndetectable is transformed to ObjectIsUndetectable operator when
building
turbofan graph.

BUG=v8:4280
==========

to

==========
[Interpreter] Optimize equality check with null/undefined with a check on the
map.

Equality with null/undefined is equivalent to a check on the undetectable bit
on the map of the object. This would be more efficient than performing the
entire
comparison operation.

This cl introduces:
1. A new bytecode called TestUndetectable that checks if the object is
null/undefined.
2. Updates peeophole optimizer to emit TestUndetectable when a LdaNull/Undefined
precedes equality check.
4. TestUndetectable is transformed to ObjectIsUndetectable operator when
building
turbofan graph.

BUG=v8:4280

Committed: https://crrev.com/9119d16904e9062d319427029c2c74270776e68e
Cr-Commit-Position: refs/heads/master@{#41514}
==========

[commit-bot: I haz the power, 2016-12-06 11:32:44]

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