For instance call representing numerical comparisons (double and Smi for now) that were never visit…

For instance call representing numerical comparisons and binary operations (double and Smi supported for now) that were never visited before optimization occurs, we guess the cids if one of the argument cids is known, e.g. ‘a == 1’ we guess that a is also going to be 1 thus avoiding emitting an IC-call in optimized code and propagating the type accordingly.

R=fschneider@google.com

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

[Florian Schneider, 2013-12-06 10:15:28]

LGTM.

Please make sure that the code cannot trigger repeated deoptimization by adding
a test that deoptimizes on a speculatively guessed cid. In other places I used
ICData::deopt_reason to guard against repeated deoptimization.

https://codereview.chromium.org/104893003/diff/20001/tests/language/guess_cid...
File tests/language/guess_cid_test.dart (right):

https://codereview.chromium.org/104893003/diff/20001/tests/language/guess_cid...
tests/language/guess_cid_test.dart:15: Expect.equals(i.toDouble(),
binOpInt(i.toDouble(), i.toDouble()));
s/binOpInt/binOpDouble/ ?

https://codereview.chromium.org/104893003/diff/20001/tests/language/guess_cid...
tests/language/guess_cid_test.dart:22: Expect.equals(0, binOpInt(-2.0, -2.0));
binOpDouble?

https://codereview.chromium.org/104893003/diff/20001/tests/language/guess_cid...
tests/language/guess_cid_test.dart:24: Expect.equals(3.0, binOpInt(3.0, 3.0));
binOpDouble?

https://codereview.chromium.org/104893003/diff/20001/tests/language/guess_cid...
tests/language/guess_cid_test.dart:56: // Guess cid of comparison below.
s/comparison/binary operation/

https://codereview.chromium.org/104893003/diff/20001/tests/language/guess_cid...
tests/language/guess_cid_test.dart:62: double binOpDouble(double i, double x) {
This function is not called in the test yet.

https://codereview.chromium.org/104893003/diff/20001/tests/language/guess_cid...
tests/language/guess_cid_test.dart:65: // Guess cid of comparison below.
s/comparison/binary operation/

[srdjan, 2013-12-10 01:02:49]

Added exact type for Mint operations. This optimization triggered a bug where
type propagator assumes Mint operations return Smi-s (subtype of Int).

https://codereview.chromium.org/104893003/diff/20001/tests/language/guess_cid...
File tests/language/guess_cid_test.dart (right):

https://codereview.chromium.org/104893003/diff/20001/tests/language/guess_cid...
tests/language/guess_cid_test.dart:15: Expect.equals(i.toDouble(),
binOpInt(i.toDouble(), i.toDouble()));
On 2013/12/06 10:15:28, Florian Schneider wrote:
> s/binOpInt/binOpDouble/ ?

Done.

https://codereview.chromium.org/104893003/diff/20001/tests/language/guess_cid...
tests/language/guess_cid_test.dart:22: Expect.equals(0, binOpInt(-2.0, -2.0));
On 2013/12/06 10:15:28, Florian Schneider wrote:
> binOpDouble?

Done.

https://codereview.chromium.org/104893003/diff/20001/tests/language/guess_cid...
tests/language/guess_cid_test.dart:24: Expect.equals(3.0, binOpInt(3.0, 3.0));
On 2013/12/06 10:15:28, Florian Schneider wrote:
> binOpDouble?

Done.

https://codereview.chromium.org/104893003/diff/20001/tests/language/guess_cid...
tests/language/guess_cid_test.dart:56: // Guess cid of comparison below.
On 2013/12/06 10:15:28, Florian Schneider wrote:
> s/comparison/binary operation/

Done.

https://codereview.chromium.org/104893003/diff/20001/tests/language/guess_cid...
tests/language/guess_cid_test.dart:62: double binOpDouble(double i, double x) {
On 2013/12/06 10:15:28, Florian Schneider wrote:
> This function is not called in the test yet.

Done.

https://codereview.chromium.org/104893003/diff/20001/tests/language/guess_cid...
tests/language/guess_cid_test.dart:65: // Guess cid of comparison below.
On 2013/12/06 10:15:28, Florian Schneider wrote:
> s/comparison/binary operation/

Done.

[srdjan, 2013-12-10 19:09:50]

Committed patchset #10 manually as r31036 (presubmit successful).

[srdjan, 2013-12-10 22:56:30]

https://codereview.chromium.org/104893003/diff/180001/runtime/vm/flow_graph_t...
File runtime/vm/flow_graph_type_propagator.cc (right):

https://codereview.chromium.org/104893003/diff/180001/runtime/vm/flow_graph_t...
runtime/vm/flow_graph_type_propagator.cc:1014: return
CompileType::FromCid(kMintCid);
Unfortunately this is incorrect, since a Mint result may produce a Smi (with
help of a BoxIntegerInstr that is appended).