dart2js cps: Better runtime null checks for primitive operators.

dart2js cps: Better runtime null checks for primitive operators.

Calls to numeric operators where the receiver might be null will now
insert a specialized null check, like this:

    if (typeof x !== 'number') return x.$lt();

BUG=
R=sra@google.com

Committed: https://github.com/dart-lang/sdk/commit/94ed2c3a7d6c844d926364cc85154bc6ed374973

[asgerf, 2015-11-27 17:11:52]

asgerf@google.com changed reviewers:
+ sra@google.com

[sra1, 2015-11-27 18:34:38]

lgtm

Like with bounds checking, we should probably add a single primitive to
summarize the test+branch+continuations+invoke+unreachable+refinement graph

https://codereview.chromium.org/1474373002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/type_propagation.dart (right):

https://codereview.chromium.org/1474373002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:1046: // This must be done
even if trusting primitives.
Why? Is there an example where hoisting a trusted operation causes different
behaviour?

I can understand this is necessary if we had a way to paint some code with
--trust-primitives and have other code untrusted.

https://codereview.chromium.org/1474373002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:1048: new
Refinement(node.dartReceiver, typeSystem.nonNullType));
Should we replace dominated uses of node.dartReceiver with the refinement to
avoid adding the guard graph twice for  [a < n, a + 1] ?

[asgerf, 2015-11-30 11:07:20]

I agree about having one instruction for the check. I've added it to our
worklist.

https://codereview.chromium.org/1474373002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/type_propagation.dart (right):

https://codereview.chromium.org/1474373002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:1046: // This must be done
even if trusting primitives.
On 2015/11/27 18:34:38, sra1 wrote:
> Why? Is there an example where hoisting a trusted operation causes different
> behaviour?
> 
> I can understand this is necessary if we had a way to paint some code with
> --trust-primitives and have other code untrusted.

If there is an "x + y" expression in a loop, where x is int|null we can trust
that x is in fact int at the point of the + operation.  But that does not mean x
is always an int outside the loop.  It might be hoisted past a condition that
the user knows guarantees non-nullness.

E.g.

foo(List list, {bool flag, int x}) {
  var sum = 0;
  for (...) {
    ...
    if (flag) {
      sum += list[x + 1];
    }
  }
  return sum;
}

Hoisting list[x + 1] is not safe.

https://codereview.chromium.org/1474373002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:1048: new
Refinement(node.dartReceiver, typeSystem.nonNullType));
On 2015/11/27 18:34:38, sra1 wrote:
> Should we replace dominated uses of node.dartReceiver with the refinement to
> avoid adding the guard graph twice for  [a < n, a + 1] ?

We don't have to, but if we want to avoid duplicate refinements I think the
better way is to find and re-use the existing refinement node instead of
creating a new one in the first place.

[asgerf, 2015-11-30 11:08:42]

Description was changed from

==========
dart2js cps: Better runtime null checks for primitive operators.

Calls to numeric operators where the receiver might be null will now
insert a specialized null check, like this:

    if (typeof x !== 'number') return x.$lt();

BUG=
==========

to

==========
dart2js cps: Better runtime null checks for primitive operators.

Calls to numeric operators where the receiver might be null will now
insert a specialized null check, like this:

    if (typeof x !== 'number') return x.$lt();

BUG=
R=sra@google.com

Committed:
https://github.com/dart-lang/sdk/commit/94ed2c3a7d6c844d926364cc85154bc6ed374973
==========

[asgerf, 2015-11-30 11:08:43]

Committed patchset #1 (id:1) manually as
94ed2c3a7d6c844d926364cc85154bc6ed374973 (presubmit successful).