AOT: Fix bug in receiver type propagation after unique selector calls.

runtime/vm/aot_optimizer.cc

Index: runtime/vm/aot_optimizer.cc
diff --git a/runtime/vm/aot_optimizer.cc b/runtime/vm/aot_optimizer.cc
index 168471ceeb93252db7dae73defb027ad0ff5ddd1..814f512455164038d44e0c4c20be0d4d8514f86d 100644
--- a/runtime/vm/aot_optimizer.cc
+++ b/runtime/vm/aot_optimizer.cc
@@ -291,6 +291,15 @@ bool AotOptimizer::TryCreateICData(InstanceCallInstr* call) {
         call->set_ic_data(&ic_data);
         if (has_unique_no_such_method_) {
           call->set_has_unique_selector(true);
+          // Add redefinition of the receiver to prevent code motion across

[Vyacheslav Egorov (Google), 2016/11/16 10:16:34]

I don't think renaming here is "future proof", though I think it works right
now. 

My main concern is that SSA value might have unrenamed aliases which are later
removed by optimization passes like load forwarding. 

For example:

foofoo(obj) {
  final objAlias = obj;
  closure() => objAlias;
  var res = 0;
  for (var i = 0; i < 2; i++) {
    obj._uniqueSelector();
    res += objAlias.uniqueField;
  }
  return res;
}

Here we will not rename anything and later store-to-load forwarding will replace
objAlias inside the loop with obj. 

Loop body before the LICM looks like this:

B3[target]:28
    PushArgument(v3)
    PolymorphicInstanceCall:6( _uniqueSelector@15186008, v3 IC[1: A cnt:1
trgt:'A._uniqueSelector@15186008'] WITH-CHECKS)
    v27 <- Redefinition(v3 T{A}) T{A}
    v18 <- LoadField(v4, 24, immutable=1)
    PushArgument(v18)
    v20 <- PolymorphicInstanceCall:8( get:uniqueField, v18 IC[1: A cnt:1
trgt:'A.get_uniqueField'] WITH-CHECKS)

So if we happen to run CSE before LICM then things will break again: because v18
will be replaced by v3 not with v27.

I think it's more future-proof to insert redifitions in the type propagator.
Additionally type propagator does traverse the dominator tree so it can rename
uses as it goes.

[Florian Schneider, 2016/11/16 17:55:18]

Agree. I remember we had this issue already in a different context where we
should insert redefinitions during type propagation.

I'll add this test case for now. We don't propagate types through aliases,
that's what currently prevents get:uniqueField from being inlined.

+          // this call.
+          RedefinitionInstr* redefinition =
+              new (Z) RedefinitionInstr(new (Z) Value(call->ArgumentAt(0)));
+          redefinition->set_ssa_temp_index(flow_graph_->alloc_ssa_temp_index());
+          redefinition->InsertAfter(call);
+          // Replace all uses of the receiver dominated by this call.
+          FlowGraph::RenameDominatedUses(call->ArgumentAt(0), redefinition,

[Vyacheslav Egorov (Google), 2016/11/16 10:16:34]

There might be no dominated uses. Maybe check that redefinition after this - if
it does not have any uses, remove it from the graph (or better allocate
redefinition lazily - when the first use to rename is found). 

[Florian Schneider, 2016/11/16 17:55:18]

Done. For now, I'll remove the redefinition eagerly if there are no uses here.

+                                         redefinition);
         }
         return true;
       }