cps_ir: add refinement on "success" arguments for a set of whitelisted methods

cps_ir: add refinement on "success" arguments for a set of whitelisted methods
and operators.

R=asgerf@google.com

Committed: https://github.com/dart-lang/sdk/commit/0f626270ad04651fad703c73d1f031d4ff9eb59e

[Siggi Cherem (dart-lang), 2016-01-08 23:27:33]

Patchset #1 (id:1) has been deleted

[Siggi Cherem (dart-lang), 2016-01-08 23:29:36]

Patchset #1 (id:20001) has been deleted

[Siggi Cherem (dart-lang), 2016-01-08 23:31:16]

Patchset #1 (id:40001) has been deleted

[Siggi Cherem (dart-lang), 2016-01-08 23:37:05]

sigmund@google.com changed reviewers:
+ asgerf@google.com, kmillikin@google.com, sra@google.com

[Siggi Cherem (dart-lang), 2016-01-08 23:37:05]

I did not see much benefit from this change on our benchmarks (just minor
changes in the output, but not much).

I added some unit tests, but I wonder if there is a better way to test this. I
just noticed that Asger disabled most of the other unittests because they didn't
add much value and it was more work to keep the tests up to date than anything.
Any other ideas?

[asgerf, 2016-01-09 00:10:14]

FYI there is an issue in the size heuristic in the inliner.

It counts refinement nodes as part of the size of a node, so that's biasing the
performance result.

[asgerf, 2016-01-11 16:33:11]

LGTM

https://codereview.chromium.org/1571953002/diff/60001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/insert_refinements.dart (right):

https://codereview.chromium.org/1571953002/diff/60001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/insert_refinements.dart:112: InvocationPrimitive
node, List<TypeMask> argumentSuccessTypes) {
I think the dartArgument method from InvokeMethod should be lifted to
InvocationPrimitive can used here instead of the offset computation.

https://codereview.chromium.org/1571953002/diff/60001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/insert_refinements.dart:115: node.callingConvention
== CallingConvention.Intercepted) ? 1 : 0;
It looks like you have two identical conditions with || between them.

https://codereview.chromium.org/1571953002/diff/60001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/insert_refinements.dart:122: if (i + offset >=
node.arguments.length) continue;
continue -> break

https://codereview.chromium.org/1571953002/diff/60001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/insert_refinements.dart:287: TypeMaskSystem types,
TypeMask receiver, Selector selector) {
Add a return type please.

[Siggi Cherem (dart-lang), 2016-01-11 18:43:29]

Patchset #2 (id:80001) has been deleted

[Siggi Cherem (dart-lang), 2016-01-11 18:43:33]

Patchset #2 (id:100001) has been deleted

[Siggi Cherem (dart-lang), 2016-01-11 18:45:37]

thanks!

https://codereview.chromium.org/1571953002/diff/60001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/insert_refinements.dart (right):

https://codereview.chromium.org/1571953002/diff/60001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/insert_refinements.dart:112: InvocationPrimitive
node, List<TypeMask> argumentSuccessTypes) {
On 2016/01/11 16:33:11, asgerf wrote:
> I think the dartArgument method from InvokeMethod should be lifted to
> InvocationPrimitive can used here instead of the offset computation.

good idea. done

https://codereview.chromium.org/1571953002/diff/60001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/insert_refinements.dart:115: node.callingConvention
== CallingConvention.Intercepted) ? 1 : 0;
On 2016/01/11 16:33:11, asgerf wrote:
> It looks like you have two identical conditions with || between them.

This was meant to be DummyIntercepted, bad copy paste :( -- another good reason
to put this all in InvocationPrimitive :)

https://codereview.chromium.org/1571953002/diff/60001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/insert_refinements.dart:122: if (i + offset >=
node.arguments.length) continue;
On 2016/01/11 16:33:11, asgerf wrote:
> continue -> break

Done.

https://codereview.chromium.org/1571953002/diff/60001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/insert_refinements.dart:287: TypeMaskSystem types,
TypeMask receiver, Selector selector) {
On 2016/01/11 16:33:11, asgerf wrote:
> Add a return type please.

Done.

https://codereview.chromium.org/1571953002/diff/120001/pkg/compiler/lib/src/c...
File pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart (right):

https://codereview.chromium.org/1571953002/diff/120001/pkg/compiler/lib/src/c...
pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart:530: int get dartArgumentsLength
=>
FYI - I also added dartArgumentsLength to expose the actual number of arguments
as well.

[Siggi Cherem (dart-lang), 2016-01-11 18:48:18]

Description was changed from

==========
cps_ir: add refinement on "success" arguments for a set of whitelisted methods
and operators.
==========

to

==========
cps_ir: add refinement on "success" arguments for a set of whitelisted methods
and operators.

R=asgerf@google.com

Committed:
https://github.com/dart-lang/sdk/commit/0f626270ad04651fad703c73d1f031d4ff9eb59e
==========

[Siggi Cherem (dart-lang), 2016-01-11 18:48:18]

Committed patchset #2 (id:120001) manually as
0f626270ad04651fad703c73d1f031d4ff9eb59e (presubmit successful).