Use short-hand update syntax for simple updating definitions.

Use short-hand update syntax for simple updating definitions.

For example turn 'i = i + 1' into '++i' and 'i = i / 0.2' into
'i /= 0.2'.

Additionally, only move equivalent instructions to a dominator block
if there is more than one successor. There is no reason to move if
there is only one in any case. This has the added benefit that the
update part of a simple for-loop stays in the update part in the
generated code.

Before:

  for (var i = 0; i < 10; ) {
    i = i + 1;
  }

Now:

  for (var i = 0; i < 10; ++i) {
  }

This code is a bit hackish. Please let me know if you have suggestions
for better structure of the cod.

R=ngeoffray@google.com,kasperl@google.com
BUG=
TEST=

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

[kasperl, 2012-06-04 13:12:56]

LGTM!

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:496: // Extract the operation and
whether or not it is commutative.
I guess you considered moving some of this code to the nodes, but that you
prefer to keep it local to the codegen.

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:498: var isCommutative =
instruction is HAdd || instruction is HMultiply;
Make this start out by being false and push the update to true inside the
branches? Your call.

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:529: if ((operation == '+') &&
right.constant.value == 1) {
Looks a bit fishy that you have () around one == (left) but not the other.

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:532: } else if ((operation == '-')
&& right.constant.value == 1) {
Ditto.

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:548:
variableNames.getName(instruction))) {
I would throw variableNames.getName(instruction) into a local to make it easier
to format.

[Mads Ager (google), 2012-06-04 13:21:45]

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:496: // Extract the operation and
whether or not it is commutative.
On 2012/06/04 13:12:56, kasperl wrote:
> I guess you considered moving some of this code to the nodes, but that you
> prefer to keep it local to the codegen.

Yes, I decided to keep it local since I only consider a few of the operations
here.

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:498: var isCommutative =
instruction is HAdd || instruction is HMultiply;
On 2012/06/04 13:12:56, kasperl wrote:
> Make this start out by being false and push the update to true inside the
> branches? Your call.

Yes! Done.

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:529: if ((operation == '+') &&
right.constant.value == 1) {
On 2012/06/04 13:12:56, kasperl wrote:
> Looks a bit fishy that you have () around one == (left) but not the other.

Yes, that looks weird. Updated to consistently use parens.

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:532: } else if ((operation == '-')
&& right.constant.value == 1) {
On 2012/06/04 13:12:56, kasperl wrote:
> Ditto.

Thanks. Done.

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:548:
variableNames.getName(instruction))) {
On 2012/06/04 13:12:56, kasperl wrote:
> I would throw variableNames.getName(instruction) into a local to make it
easier
> to format.

Done.

[ngeoffray, 2012-06-04 13:24:00]

LGTM

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:494: if (!isVariableDeclared(name))
return false;
I think you should check that it's not delayed either. It should be fine to use
i++ if it's delayed. Or maybe should we declare a variable as declared if it's
delayed? I think we're duplicating 'var' if we don't.

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:497: var operation;
Please use the Operation class in lib/compiler/implementation/operations.dart to
check for commutativity and the operation's name.

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:526: if (left is HPhi &&
I think you can remove the check. You only need to check if
variableNames.getName(left) == name

[Mads Ager (google), 2012-06-04 15:27:18]

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:494: if (!isVariableDeclared(name))
return false;
On 2012/06/04 13:24:00, ngeoffray wrote:
> I think you should check that it's not delayed either. It should be fine to
use
> i++ if it's delayed. Or maybe should we declare a variable as declared if it's
> delayed? I think we're duplicating 'var' if we don't.

Yes, went for marking a variable declared when we add it to the
delayedVariables.

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:497: var operation;
On 2012/06/04 13:24:00, ngeoffray wrote:
> Please use the Operation class in lib/compiler/implementation/operations.dart
to
> check for commutativity and the operation's name.

In principle that would be very nice. In practice the Operation class is not
very well suited for this. Add operations are not commutative when dealing with
strings so we need a special case for that. Not all operations that are
BinaryArithmetic operations and are builtin will work with this (such as
truncating divide). So, in the end we will end up with the same checks. I have
used the Operation class to get the name of the operation which makes the code
shorter, but I still need the other checks.

https://chromiumcodereview.appspot.com/10520003/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:526: if (left is HPhi &&
On 2012/06/04 13:24:00, ngeoffray wrote:
> I think you can remove the check. You only need to check if
> variableNames.getName(left) == name

Great, thanks!