Materialize all constant operands on the stack in the non-optimizing compiler.

Materialize all constant operands on the stack in the non-optimizing compiler.

This eliminates the distinction between 'value' and 'argument value'.

R=srdjan@google.com
BUG=
TEST=

Committed: https://code.google.com/p/dart/source/detail?r=7181

[Kevin Millikin (Google), 2012-04-30 14:54:52]

http://codereview.chromium.org/10264011/diff/1/runtime/vm/flow_graph_builder.cc
File runtime/vm/flow_graph_builder.cc (right):

http://codereview.chromium.org/10264011/diff/1/runtime/vm/flow_graph_builder....
runtime/vm/flow_graph_builder.cc:278: if ((value != NULL) &&
value->IsLiteralNode()) {
We still need this constant check, because in the case of the sentinels below,
we not only can but *must* skip the type check.  :)

http://codereview.chromium.org/10264011/diff/1/runtime/vm/flow_graph_builder....
runtime/vm/flow_graph_builder.cc:321: node->expr(),
This changes the token index from node->token_index() to
node->expr()->token_index().  I think that's OK and matches the pattern we have
everywhere else.

http://codereview.chromium.org/10264011/diff/1/runtime/vm/flow_graph_compiler...
File runtime/vm/flow_graph_compiler_x64.cc (left):

http://codereview.chromium.org/10264011/diff/1/runtime/vm/flow_graph_compiler...
runtime/vm/flow_graph_compiler_x64.cc:1229: #ifdef DEBUG
This assert doesn't work anymore because we can have un-popped expression values
on the stack when we return.  Example:

try {
  return 0;
} finally {
  return 1;
}

The value of 0 is sitting on the stack at the return from the finally.  (It
didn't really work before either).

We can reintroduce it when (if) we add stack height tracking and assert the
expected height.

[srdjan, 2012-05-01 00:56:10]

LGTM

http://codereview.chromium.org/10264011/diff/1/runtime/vm/flow_graph_compiler...
File runtime/vm/flow_graph_compiler_x64.cc (left):

http://codereview.chromium.org/10264011/diff/1/runtime/vm/flow_graph_compiler...
runtime/vm/flow_graph_compiler_x64.cc:1229: #ifdef DEBUG
On 2012/04/30 14:54:53, kmillikin wrote:
> This assert doesn't work anymore because we can have un-popped expression
values
> on the stack when we return.  Example:
> 
> try {
>   return 0;
> } finally {
>   return 1;
> }
> 
> The value of 0 is sitting on the stack at the return from the finally.  (It
> didn't really work before either).
> 
> We can reintroduce it when (if) we add stack height tracking and assert the
> expected height.

This assert code is/was essential for tracking/locating bugs so far (catches
bugs that otherwise wold go unnoticed). There is difference between what we
track and what the reality is.

Is there a way that we could keep it? Maybe disable it temporarily for now?