Add an optional parameter to the dominatedUsers function in case we want to analyze the whole block,

Add an optional parameter to the dominatedUsers function in case we want to analyze the whole block, and not only from an instruction. Fixes bug 4069.
Committed: https://code.google.com/p/dart/source/detail?r=10957

[kasperl, 2012-08-20 11:37:21]

LGTM, but maybe you could get away with always adding [other] to the set if it
is a user?

Or pass a boolean flag to the function telling whether or not you want [other]
to be included or not (if it is a user)?

https://chromiumcodereview.appspot.com/10824371/diff/3/lib/compiler/implement...
File lib/compiler/implementation/ssa/nodes.dart (right):

https://chromiumcodereview.appspot.com/10824371/diff/3/lib/compiler/implement...
lib/compiler/implementation/ssa/nodes.dart:936: Set<HInstruction>
dominatedUsers(HInstruction other,
I'd prefer two different entry points for this: 

   1. usersDominatedByInstruction(HInstruction)
   2. usersDominatedByBlock(HBlock)

The current method might be helper function for implementing the two functions
and you can assert that we're not passing in both an instruction and a block.

https://chromiumcodereview.appspot.com/10824371/diff/3/lib/compiler/implement...
lib/compiler/implementation/ssa/nodes.dart:945: // potentially dominated by
[otherBlock].
Here you're adding all the users that are dominated by the entry instruction in
the otherBlock, so you should probably update the comment to not contain "or
potentially dominated" because they all are dominated by otherBlock.

https://chromiumcodereview.appspot.com/10824371/diff/3/lib/compiler/implement...
lib/compiler/implementation/ssa/nodes.dart:965: // Run through all the
instructions before otherBlock and remove them
This should still be all instructions before [other], right?

https://chromiumcodereview.appspot.com/10824371/diff/3/lib/compiler/implement...
File lib/compiler/implementation/ssa/optimize.dart (right):

https://chromiumcodereview.appspot.com/10824371/diff/3/lib/compiler/implement...
lib/compiler/implementation/ssa/optimize.dart:1133: // Update users of [input]
that are dominated by [:dominator.first:]
Update comment.

https://chromiumcodereview.appspot.com/10824371/diff/3/lib/compiler/implement...
lib/compiler/implementation/ssa/optimize.dart:1140: for (HInstruction user in
dominatedUsers) {
How about returning if dominatedUsers.isEmpty and then just creating newInput
right away (outside the loop)? Seems cleaner than checking if newInput is null
inside the loop.

[ngeoffray, 2012-08-20 12:55:01]

Thank you Kasper for your comments. I have changed the code to include [other]
if it's in the list of users, and changed all call sites affected. PTAL.

[kasperl, 2012-08-20 12:58:35]

LGTM!

https://chromiumcodereview.appspot.com/10824371/diff/1002/lib/compiler/implem...
File lib/compiler/implementation/ssa/nodes.dart (right):

https://chromiumcodereview.appspot.com/10824371/diff/1002/lib/compiler/implem...
lib/compiler/implementation/ssa/nodes.dart:935: // [other], [other] included.
Maybe explain this a bit better. Like: If [other] is a user of [this]
instruction, it is included in the returned set.

https://chromiumcodereview.appspot.com/10824371/diff/1002/lib/compiler/implem...
File lib/compiler/implementation/ssa/optimize.dart (right):

https://chromiumcodereview.appspot.com/10824371/diff/1002/lib/compiler/implem...
lib/compiler/implementation/ssa/optimize.dart:1143: for (HInstruction user in
dominatedUsers) {
Use dominatedUsers.forEach?

[ngeoffray, 2012-08-20 13:01:32]

Thanks Kasper

https://chromiumcodereview.appspot.com/10824371/diff/1002/lib/compiler/implem...
File lib/compiler/implementation/ssa/nodes.dart (right):

https://chromiumcodereview.appspot.com/10824371/diff/1002/lib/compiler/implem...
lib/compiler/implementation/ssa/nodes.dart:935: // [other], [other] included.
On 2012/08/20 12:58:35, kasperl wrote:
> Maybe explain this a bit better. Like: If [other] is a user of [this]
> instruction, it is included in the returned set.

Done.

https://chromiumcodereview.appspot.com/10824371/diff/1002/lib/compiler/implem...
File lib/compiler/implementation/ssa/optimize.dart (right):

https://chromiumcodereview.appspot.com/10824371/diff/1002/lib/compiler/implem...
lib/compiler/implementation/ssa/optimize.dart:1143: for (HInstruction user in
dominatedUsers) {
On 2012/08/20 12:58:35, kasperl wrote:
> Use dominatedUsers.forEach?

Sure. Personal preference?