When canonicalizing branch on StrictCompare ensure that branch gets correct environment.

When canonicalizing branch on StrictCompare don't fuse comparisons that can deoptimize or serve as pending deoptimization target for representation changes.

R=fschneider@google.com
BUG=

Committed: https://code.google.com/p/dart/source/detail?r=18443

[Florian Schneider, 2013-02-13 11:22:11]

LGTM.

https://codereview.chromium.org/12220150/diff/1/runtime/vm/intermediate_langu...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/12220150/diff/1/runtime/vm/intermediate_langu...
runtime/vm/intermediate_language.cc:1845: comp->set_env(NULL);
You may also need to set a new deopt id on the environment for consistency:

env()->set_deopt_id(deopt_id())

https://codereview.chromium.org/12220150/diff/1/tests/language/branch_canonic...
File tests/language/branch_canonicalization_test.dart (right):

https://codereview.chromium.org/12220150/diff/1/tests/language/branch_canonic...
tests/language/branch_canonicalization_test.dart:18: Expect.equals(0, fooMint(b,
0));
Thanks for adding the test. Do we have a test covering the case where the
comparison itself can deoptimize? (e.g. as in EmitEqualityAsPolymorphicCall)

[Vyacheslav Egorov (Google), 2013-02-13 13:46:23]

please take another look.

I completely redid this CL addressing the issue with incorrect deoptimization
target.

Tests are improved to cover problematic cases.

[Florian Schneider, 2013-02-13 14:08:43]

new patch LGTM.

https://codereview.chromium.org/12220150/diff/8001/runtime/vm/intermediate_la...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/12220150/diff/8001/runtime/vm/intermediate_la...
runtime/vm/intermediate_language.cc:1830:
comp->InputAt(i)->definition()->representation()) {
Indentation off.

https://codereview.chromium.org/12220150/diff/8001/tests/language/branch_cano...
File tests/language/branch_canonicalization_test.dart (right):

https://codereview.chromium.org/12220150/diff/8001/tests/language/branch_cano...
tests/language/branch_canonicalization_test.dart:19: final result = (a == b);
Why "final" here and "var" below in barPoly?

[Vyacheslav Egorov (Google), 2013-02-13 14:10:19]

Thanks. Landing.

https://codereview.chromium.org/12220150/diff/8001/runtime/vm/intermediate_la...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/12220150/diff/8001/runtime/vm/intermediate_la...
runtime/vm/intermediate_language.cc:1830:
comp->InputAt(i)->definition()->representation()) {
On 2013/02/13 14:08:43, Florian Schneider wrote:
> Indentation off.

Done.

https://codereview.chromium.org/12220150/diff/8001/tests/language/branch_cano...
File tests/language/branch_canonicalization_test.dart (right):

https://codereview.chromium.org/12220150/diff/8001/tests/language/branch_cano...
tests/language/branch_canonicalization_test.dart:19: final result = (a == b);
On 2013/02/13 14:08:43, Florian Schneider wrote:
> Why "final" here and "var" below in barPoly?

cleaned up

[ahe, 2013-02-13 14:46:23]

DBC linting

https://codereview.chromium.org/12220150/diff/4003/tests/language/branch_cano...
File tests/language/branch_canonicalization_test.dart (right):

https://codereview.chromium.org/12220150/diff/4003/tests/language/branch_cano...
tests/language/branch_canonicalization_test.dart:3: // BSD-style license that
can be found in the LICENSE file.
Add empty line between copyright and file comment.

https://codereview.chromium.org/12220150/diff/4003/tests/language/branch_cano...
tests/language/branch_canonicalization_test.dart:65: 
Extra lines.

[Vyacheslav Egorov (Google), 2013-03-19 20:27:25]

sending out stale comments.

https://codereview.chromium.org/12220150/diff/4003/tests/language/branch_cano...
File tests/language/branch_canonicalization_test.dart (right):

https://codereview.chromium.org/12220150/diff/4003/tests/language/branch_cano...
tests/language/branch_canonicalization_test.dart:3: // BSD-style license that
can be found in the LICENSE file.
On 2013/02/13 14:46:23, ahe wrote:
> Add empty line between copyright and file comment.

I am not sure what are style here is. I see a lot of tests that don't have this
empty line (I am just copying the style).

https://codereview.chromium.org/12220150/diff/4003/tests/language/branch_cano...
tests/language/branch_canonicalization_test.dart:65: 
On 2013/02/13 14:46:23, ahe wrote:
> Extra lines.

I will remove this.