Implement branching control flow in the non-optimizing graph compiler.

Implement branching control flow in the non-optimizing graph compiler.

Bind a label on block entry, implement branching.

R=srdjan@google.com
BUG=
TEST=

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

[Kevin Millikin (Google), 2012-03-07 16:51:27]

There is a bug here, we timeout on the IsolateInterrupt test.  I'm nearly
positive it is in this function:


==== dart:test-lib_::_main
 0: [target]
    StaticCall(markMainEntered) goto 1
 1: [join]
    if #true goto(2, 3)
 2: [target] goto 1
 3: [target]
    return #null

Which is a very nice infinite loop.  Probably a missing stack check on the back
edge.  (By the way, if we don't insist on edge-split form, block 2 is redundant
:) ).

[srdjan, 2012-03-07 22:23:06]

LGTM.

Yes, the backward branch stack check is the reason for failing test. Feel free
to disable it in the status list and submit. (the backward loop stack check is
for being able to stop endless loops).

https://chromiumcodereview.appspot.com/9623005/diff/1/runtime/vm/flow_graph_c...
File runtime/vm/flow_graph_compiler_x64.cc (right):

https://chromiumcodereview.appspot.com/9623005/diff/1/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_x64.cc:295: int number =
BlockEntryInstr::cast(instr)->block_number();
Discussed in the team and talked with Ivan: Please use AsXXX style instead of
::cast. The former is the DartVM style the second is V8 style. We do not use the
cast-like-style, and it is confusing if we use both.

https://chromiumcodereview.appspot.com/9623005/diff/1/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_x64.cc:394: int index = blocks_->length() -
current_block()->block_number() - 1;
intptr_t

https://chromiumcodereview.appspot.com/9623005/diff/1/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_x64.cc:395: ASSERT(index != 0);
Maybe safer: (index > 0).

https://chromiumcodereview.appspot.com/9623005/diff/1/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_x64.cc:405: 
Empty line?

https://chromiumcodereview.appspot.com/9623005/diff/1/runtime/vm/flow_graph_c...
File runtime/vm/flow_graph_compiler_x64.h (right):

https://chromiumcodereview.appspot.com/9623005/diff/1/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_x64.h:23: public:
Maybe write (again) that blocks are in reversed order as that they are
generated.

https://chromiumcodereview.appspot.com/9623005/diff/1/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_x64.h:43: };
ZoneAllocated objects have their destructor never called, i.e., the Label
destructor will never be called neither, i.e., the checks in Label's destructor
are not being done any longer. I think those checks are important. Maybe we
could traverse the list at the end of compilation and verify that the Labels are
OK.

[Kevin Millikin (Google), 2012-03-08 09:59:53]

https://chromiumcodereview.appspot.com/9623005/diff/1/runtime/vm/flow_graph_c...
File runtime/vm/flow_graph_compiler_x64.cc (right):

https://chromiumcodereview.appspot.com/9623005/diff/1/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_x64.cc:295: int number =
BlockEntryInstr::cast(instr)->block_number();
On 2012/03/07 22:23:06, srdjan wrote:
> Discussed in the team and talked with Ivan: Please use AsXXX style instead of
> ::cast. The former is the DartVM style the second is V8 style. We do not use
the
> cast-like-style, and it is confusing if we use both.

Removed the static member function from class BlockEntryInstr.  Rewrote it with
As.

https://chromiumcodereview.appspot.com/9623005/diff/1/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_x64.cc:394: int index = blocks_->length() -
current_block()->block_number() - 1;
On 2012/03/07 22:23:06, srdjan wrote:
> intptr_t

Done.

https://chromiumcodereview.appspot.com/9623005/diff/1/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_x64.cc:395: ASSERT(index != 0);
On 2012/03/07 22:23:06, srdjan wrote:
> Maybe safer: (index > 0).

Done.

https://chromiumcodereview.appspot.com/9623005/diff/1/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_x64.cc:405: 
On 2012/03/07 22:23:06, srdjan wrote:
> Empty line?

Deleted.

https://chromiumcodereview.appspot.com/9623005/diff/1/runtime/vm/flow_graph_c...
File runtime/vm/flow_graph_compiler_x64.h (right):

https://chromiumcodereview.appspot.com/9623005/diff/1/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_x64.h:43: };
On 2012/03/07 22:23:06, srdjan wrote:
> ZoneAllocated objects have their destructor never called, i.e., the Label
> destructor will never be called neither, i.e., the checks in Label's
destructor
> are not being done any longer. I think those checks are important. Maybe we
> could traverse the list at the end of compilation and verify that the Labels
are
> OK.

I know, and I'm not certain this is best.  Label has DISALLOW_COPY_AND_ASSIGN so
it doesn't really work to use an embedded GrowableArray.

It seems possibly better to have a malloced array of BlockInfos---especially
depending on what else we put in this struct.

For now I've put the label asserts in ~FlowGraphCompiler.