Refactoring if block-information.

Refactoring if block-information.

Generalizes block information handling and adds (yet unused) info blocks
for if and logical-and/or.

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

[floitsch, 2012-04-16 13:26:07]

LGTM.

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

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:417: if (isCondition(condition)) {
I would prefer bailing out here.

if (!isCondition(condition)) return false;
if (info.type == HLoopInformation.DO_WHILE_LOOP) return false;

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:418: switch (info.type) {
I would prefer 'kind' instead of 'type'.

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:420: case
HLoopInformation.FOR_IN_LOOP: {
Afaics there is almost no difference between while, for-in, and for. Why is the
for-loop treated specially?

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:492: // Currently unhandled.
We don't have fall-throughs. Why didn't this through an error?

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:493: default:
Is default an internal error?

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:496: if (success) {
Unless wrong you can merge while, for-in and for. But even if you can't, please
move this into the respective cases.

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:626: // Even if our special
handling didn't succeed, loop blocks
If our special handling didn't succeed, we have to emit a generic version. There
is hence still special handling for loop-blocks.

[ngeoffray, 2012-04-17 07:44:20]

DBC

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

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:1376: visitIf(If node) {
You can remove the five lines below

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:2726: // Don't actually bother
testing the expressions.
The comment and the line below don't seem to match.

[Lasse Reichstein Nielsen, 2012-04-17 12:58:22]

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

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:1376: visitIf(If node) {
Well spotted.

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:2726: // Don't actually bother
testing the expressions.
Reworded.

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

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:417: if (isCondition(condition)) {
On 2012/04/16 13:26:07, floitsch wrote:
> I would prefer bailing out here.
> 
> if (!isCondition(condition)) return false;
> if (info.type == HLoopInformation.DO_WHILE_LOOP) return false;

Done.

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:418: switch (info.type) {
On 2012/04/16 13:26:07, floitsch wrote:
> I would prefer 'kind' instead of 'type'.

Done.

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:420: case
HLoopInformation.FOR_IN_LOOP: {
It can have an update block that isn't just phis.
And an initialization part, that I want to put in the for-structure eventually.
But true, the difference isn't big.

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:492: // Currently unhandled.
It's an empty case, so it's joined with the following default.
Comments are not significant.

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:493: default:
Probably, yes. I'll make it an internal error, and just return false from
do-while.

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:496: if (success) {
I'll keep it here, but without the if (I'll return false immediately when I bail
out).

https://chromiumcodereview.appspot.com/10100001/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:626: // Even if our special
handling didn't succeed, loop blocks
On 2012/04/16 13:26:07, floitsch wrote:
> If our special handling didn't succeed, we have to emit a generic version.
There
> is hence still special handling for loop-blocks.

Done.