New CL for https://chromiumcodereview.appspot.com/9784002/: Support non-speculative type propagatio…

New CL for https://chromiumcodereview.appspot.com/9784002/: Support non-speculative type propagation.
Committed: https://code.google.com/p/dart/source/detail?r=5969

[ngeoffray, 2012-03-28 07:24:45]

TBR, code was already reviewed here:
 https://chromiumcodereview.appspot.com/9784002/

[ngeoffray, 2012-03-29 09:09:20]

Due to a conflicting change that revealed a bug in this change, I did not commit
it. PTAL.

[Lasse Reichstein Nielsen, 2012-03-29 11:46:55]

LGTM

https://chromiumcodereview.appspot.com/9874014/diff/3001/lib/compiler/impleme...
File lib/compiler/implementation/ssa/optimize.dart (right):

https://chromiumcodereview.appspot.com/9874014/diff/3001/lib/compiler/impleme...
lib/compiler/implementation/ssa/optimize.dart:43: new
SsaTypePropagator(compiler, true),
Make the boolean parameter optional so you can give it a name here.
Booleans are magic constants too :)

https://chromiumcodereview.appspot.com/9874014/diff/3001/lib/compiler/impleme...
lib/compiler/implementation/ssa/optimize.dart:103: // type.
Why don't we run the constant folder before type propagation?
Seems that constant folding don't need types, and it would reduce the size of
the code we type propagate on.

https://chromiumcodereview.appspot.com/9874014/diff/3001/lib/compiler/impleme...
File lib/compiler/implementation/ssa/types.dart (right):

https://chromiumcodereview.appspot.com/9874014/diff/3001/lib/compiler/impleme...
lib/compiler/implementation/ssa/types.dart:13: SsaTypePropagator(Compiler
this.compiler, bool this.speculative)
As mentioned: make this argument optional so you can name the parameter (or,
perhaps, make two constructors, one called SsaTypePropagator.speculative).

Boolean mode switches are annoying.

https://chromiumcodereview.appspot.com/9874014/diff/3001/lib/compiler/impleme...
lib/compiler/implementation/ssa/types.dart:27: if (speculative) {
Is this the only place speculative is used? 
If so, as a third option, extract this into a separate method that does nothing
in the base class, and override it in a speculative type propagator subclass.

[ngeoffray, 2012-03-29 12:51:02]

Thanks Lasse!

https://chromiumcodereview.appspot.com/9874014/diff/3001/lib/compiler/impleme...
File lib/compiler/implementation/ssa/optimize.dart (right):

https://chromiumcodereview.appspot.com/9874014/diff/3001/lib/compiler/impleme...
lib/compiler/implementation/ssa/optimize.dart:103: // type.
On 2012/03/29 11:46:55, Lasse Reichstein Nielsen wrote:
> Why don't we run the constant folder before type propagation?

No reason, I added the type propagator as the first optimization to run because
it's a trivial optimization. Constant folding could potentially make use of
types, so I'll keep it how it is right now, but we can definitely revisit the
order.

> Seems that constant folding don't need types, and it would reduce the size of
> the code we type propagate on.

It doesn't need types, but it needs to set it on the new instruction. I
understand now that the comment in the code is outdated, I'll change it :)

https://chromiumcodereview.appspot.com/9874014/diff/3001/lib/compiler/impleme...
File lib/compiler/implementation/ssa/types.dart (right):

https://chromiumcodereview.appspot.com/9874014/diff/3001/lib/compiler/impleme...
lib/compiler/implementation/ssa/types.dart:27: if (speculative) {
On 2012/03/29 11:46:55, Lasse Reichstein Nielsen wrote:
> Is this the only place speculative is used? 
> If so, as a third option, extract this into a separate method that does
nothing
> in the base class, and override it in a speculative type propagator subclass.

Done.

[floitsch, 2012-03-29 22:00:38]

https://chromiumcodereview.appspot.com/9874014/diff/3001/lib/compiler/impleme...
File lib/compiler/implementation/ssa/optimize.dart (right):

https://chromiumcodereview.appspot.com/9874014/diff/3001/lib/compiler/impleme...
lib/compiler/implementation/ssa/optimize.dart:103: // type.
On 2012/03/29 11:46:55, Lasse Reichstein Nielsen wrote:
> Why don't we run the constant folder before type propagation?
> Seems that constant folding don't need types, and it would reduce the size of
> the code we type propagate on.

The constant-folder needs types to fold away boolifications. Unless I'm wrong it
could also fold away half of the array-bounds checks when the index is known to
be a positive integer.