Add a smi-check instruction for arithmetic smi operations.

Add a smi-check instruction for arithmetic smi operations.

The flowgraph optimizer inserts smi checks for the inputs of
arithmetic smi operations:

      PushArgument v0
      PushArgument v1
v2 <- InstanceCall(+, v0, v1) IC[1: Smi, Smi]

becomes

      CheckSmi(v0)
      CheckSmi(v1)
v2 <- BinarySmiOp(+, v0, v1)


Each input operand is checked separately. This avoids using a temp
register for a combined smi check. It also allows us to easily eliminate
the checks for left and right input separately.

There are two ways to eliminate smi checks:
1. By common subexpression elimination: if the value checked is already
   checked for smi-ness before.
2. By class-id propagation: If the input value is guaranteed to be a smi.
Committed: https://code.google.com/p/dart/source/detail?r=11216

[srdjan, 2012-08-22 15:56:51]

LGTM

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/flow_graph_...
File runtime/vm/flow_graph_optimizer.cc (right):

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_optimizer.cc:342: instr->env()->Copy());
Some binary ops will now not be able to deoptimize (AND, OR, ...). Shall we
clear environment for those?

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language.cc (right):

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:1089: return (value()->ResultCid() ==
kSmiCid) ?  NULL : instr;
This is too early to eliminate all checks as FlowGraphOptimizer runs before the
type propagation (and type propagation needs to run after some nodes have been
replaced). You should have another check in CheckSmiComp::EmitNative.

Maybe we need another pass after type propagation that will eliminate all
checks?

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language.h (right):

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:157: // By default return its input parameter
(no replacement).
'its input parameter' is not that clear to me. I thought it is the computation's
input parameter being returned.

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language_ia32.cc (right):

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_ia32.cc:1469: // Can't deoptimize. Arguments
are already checked for smi.
How can we say that this computation cannot deoptimize
(Computation::CanDeoptimize) ? Different computation for the bit operations? A
flag in BinarySmiOpComp?

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_ia32.cc:2194: void
CheckSmiComp::EmitNativeCode(FlowGraphCompiler* compiler) {
You want to check here if we can skip Smi check.

[Florian Schneider, 2012-08-23 07:53:15]

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/flow_graph_...
File runtime/vm/flow_graph_optimizer.cc (right):

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_optimizer.cc:342: instr->env()->Copy());
On 2012/08/22 15:56:51, srdjan wrote:
> Some binary ops will now not be able to deoptimize (AND, OR, ...). Shall we
> clear environment for those?

Done. Yes, I forgot to return the right value in CanDeoptimize for &, |, ^
operations. When CanDeoptimize is false, the register allocator removes
environments automatically.

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language.cc (right):

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:1089: return (value()->ResultCid() ==
kSmiCid) ?  NULL : instr;
On 2012/08/22 15:56:51, srdjan wrote:
> This is too early to eliminate all checks as FlowGraphOptimizer runs before
the
> type propagation (and type propagation needs to run after some nodes have been
> replaced). You should have another check in CheckSmiComp::EmitNative.
> 
> Maybe we need another pass after type propagation that will eliminate all
> checks?

Already done. TryReplace is called by the pass OptimizeComputations which is
invoked after FlowGraphTypePropagator.

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language.h (right):

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:157: // By default return its input parameter
(no replacement).
On 2012/08/22 15:56:51, srdjan wrote:
> 'its input parameter' is not that clear to me. I thought it is the
computation's
> input parameter being returned.

I changed the comment. The old behaviour was that a NULL return value menas that
nothing changes. Now NULL means that the instructions will be removed.

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language_ia32.cc (right):

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_ia32.cc:1469: // Can't deoptimize. Arguments
are already checked for smi.
On 2012/08/22 15:56:51, srdjan wrote:
> How can we say that this computation cannot deoptimize
> (Computation::CanDeoptimize) ? Different computation for the bit operations? A
> flag in BinarySmiOpComp?

Done. Changed CanDeoptimize accordingly.

https://chromiumcodereview.appspot.com/10867012/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_ia32.cc:2194: void
CheckSmiComp::EmitNativeCode(FlowGraphCompiler* compiler) {
On 2012/08/22 15:56:51, srdjan wrote:
> You want to check here if we can skip Smi check.

Done. OptimizeComputations should eliminate all redundant checks.