Improve constant propagation to handle InstantiateTypeArguments.

Improve constant propagation to handle InstantiateTypeArguments.

If the input to an InstantiateTypeArguments instruction is a constant
it can be eliminated and replaced with the constant.
Committed: https://code.google.com/p/dart/source/detail?r=15447

[Kevin Millikin (Google), 2012-11-28 07:57:24]

Transfer function for type arguments seems subtly wrong.  LGTM with the fix
noted below.

https://codereview.chromium.org/11299204/diff/1/runtime/vm/flow_graph_optimiz...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/11299204/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:3443: if (IsNonConstant(object)) {
If the value is unknown (bottom) it should not move up the lattice to
non-constant (top) because it prevents us discovering it is constant later.

I don't personally like the early return in such a small function, either.  In
my opinion, this is clearer:

if (IsNonConstant(object)) {
  SetValue(instr, non_constant_);
} else if (IsConstant(object)) {
  if (!object.IsNull() &&
      object.IsTypeArguments() &&
      TypeArguments::Cast(object).Length() == instr->type_arguments().Length()))
{
    SetValue(instr, object);
  } else {
    SetValue(instr, non_constant_);
  }
}

[Florian Schneider, 2012-11-28 13:02:59]

https://codereview.chromium.org/11299204/diff/1/runtime/vm/flow_graph_optimiz...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/11299204/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:3443: if (IsNonConstant(object)) {
On 2012/11/28 07:57:25, kmillikin wrote:
> If the value is unknown (bottom) it should not move up the lattice to
> non-constant (top) because it prevents us discovering it is constant later.
> 
> I don't personally like the early return in such a small function, either.  In
> my opinion, this is clearer:
> 
> if (IsNonConstant(object)) {
>   SetValue(instr, non_constant_);
> } else if (IsConstant(object)) {
>   if (!object.IsNull() &&
>       object.IsTypeArguments() &&
>       TypeArguments::Cast(object).Length() ==
instr->type_arguments().Length()))
> {
>     SetValue(instr, object);
>   } else {
>     SetValue(instr, non_constant_);
>   }
> }

Thanks. Done.

[srdjan, 2012-11-30 00:49:54]

DBC

https://codereview.chromium.org/11299204/diff/1/runtime/vm/flow_graph_optimiz...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/11299204/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:3447: if (!object.IsNull() &&
object.IsTypeArguments()) {
You need to handle also the case for null otherwise "new List()" is not
optimized. (check generated code for "new List()" vs "new List<int>()").

[Florian Schneider, 2012-12-04 11:40:09]

https://codereview.chromium.org/11299204/diff/1/runtime/vm/flow_graph_optimiz...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/11299204/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:3447: if (!object.IsNull() &&
object.IsTypeArguments()) {
On 2012/11/30 00:49:54, srdjan wrote:
> You need to handle also the case for null otherwise "new List()" is not
> optimized. (check generated code for "new List()" vs "new List<int>()").

Yes, that's a good suggestion. I'll add that case.