dart2js cps: Set a flag on InvokeMethod when the receiver is intercepted

dart2js cps: Set a flag on InvokeMethod when the receiver is intercepted

This should make it clear where the "actual receiver" of a method call
can be found after dummy receiver optimization.

BUG=
R=karlklose@google.com, sra@google.com

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

[asgerf, 2015-09-11 11:53:10]

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

[asgerf, 2015-09-11 12:05:03]

On 2015/09/11 11:53:10, asgerf wrote:

This is sort of a follow-up on the synthetic dummy receiver constant issue.

As far as I understand the purpose of the dummy receiver constant is to preserve
the type of argument[0] so other optimizations can still trust that
argument[0].type
on an intercepted call is the actual type of the receiver.

The problem with the dummy constant is that we can get a constant C whose type
is another constant C'. It is not an unfixable problem, but the fix is awkward.

Please let me know if the dummy constant is needed for some other reason that is
not
covered by this CL.

[karlklose, 2015-09-11 12:09:53]

LGTM.

[sra1, 2015-09-11 15:10:48]

On 2015/09/11 12:05:03, asgerf wrote:
> On 2015/09/11 11:53:10, asgerf wrote:
> 
> This is sort of a follow-up on the synthetic dummy receiver constant issue.
> 
> As far as I understand the purpose of the dummy receiver constant is to
preserve
> the type of argument[0] so other optimizations can still trust that
> argument[0].type
> on an intercepted call is the actual type of the receiver.
> 
> The problem with the dummy constant is that we can get a constant C whose type
> is another constant C'. It is not an unfixable problem, but the fix is
awkward.

How does this happen if the constant is created for each InvokeMethod with the
receiver's refined type?

> 
> Please let me know if the dummy constant is needed for some other reason that
is
> not
> covered by this CL.

In SSA there is a use of synthetic constants of kind EMPTY_VALUE to replace phi
inputs from unreachable code. This improves type propagation ahead of removing
the phis.

[sra1, 2015-09-11 15:28:54]

LGTM.

In SSA this is handled by a predicate on the structure of the inputs. That is
fragile, so I like making it explicit.  It is robust against things we never
tried on SSA, like out-lining parts of a method.

What you might do is add something in the invariant checker.

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

https://codereview.chromium.org/1335933004/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:796: Primitive
getDartReceiver(InvokeMethod node) {
Maybe make this a member of InvokeMethod.

https://codereview.chromium.org/1335933004/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:1481: // determined in
visitInterceptor that we are targeting a non-interceptor.
FWIW I would do the dummy receiver optimization as late as possible.
All it does is remove a reference to a variable that is used in the instruction,
i.e   receiver.f(receiver) --> receiver.f(0)

The opportunity to do this could depend on strengthening of the receiver type.
Dropping the interceptor on some paths could reduce the live range:


foo(dynamic x) {
  if (x.isEmpty) return;
  if (x is Queue) {
    // There are no intercepted Queues.
    x.add(5);
    use(x);
  }
}

var x_ = getInterceptor(x);
if (x_.get$isEmpty(x)) return;
if (x_.$isQueue) {
  x_.add$1(x, 5);  //  could be: x.add$1(0, 5);
  use(x);
}

[asgerf, 2015-09-11 15:38:07]

> > The problem with the dummy constant is that we can get a constant C whose
type
> > is another constant C'. It is not an unfixable problem, but the fix is
> awkward.
> 
> How does this happen if the constant is created for each InvokeMethod with the
> receiver's refined type?

The null type (i.e. an "empty nullable TypeMask") is treated the as constant
null.

[asgerf, 2015-09-14 09:32:15]

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

https://codereview.chromium.org/1335933004/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:796: Primitive
getDartReceiver(InvokeMethod node) {
On 2015/09/11 15:28:54, sra1 wrote:
> Maybe make this a member of InvokeMethod.

I wanted to do this. It's just bothering me that we can't do that for
getDartArgument, since it depends on the backend, and it's awkward that they are
not in the same place.

https://codereview.chromium.org/1335933004/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:1481: // determined in
visitInterceptor that we are targeting a non-interceptor.
On 2015/09/11 15:28:54, sra1 wrote:
> FWIW I would do the dummy receiver optimization as late as possible.
> All it does is remove a reference to a variable that is used in the
instruction,
> i.e   receiver.f(receiver) --> receiver.f(0)
> 
> The opportunity to do this could depend on strengthening of the receiver type.
> Dropping the interceptor on some paths could reduce the live range:
> 
> 
> foo(dynamic x) {
>   if (x.isEmpty) return;
>   if (x is Queue) {
>     // There are no intercepted Queues.
>     x.add(5);
>     use(x);
>   }
> }
> 
> var x_ = getInterceptor(x);
> if (x_.get$isEmpty(x)) return;
> if (x_.$isQueue) {
>   x_.add$1(x, 5);  //  could be: x.add$1(0, 5);
>   use(x);
> }

This pass takes place after refinement nodes have been inserted, so the types
should indeed be strengthened here.

[asgerf, 2015-09-15 10:13:11]

Committed patchset #2 (id:20001) manually as
e48d774006d156038223c1ce2f0149743a389419 (presubmit successful).