Introduce Goto instructions to the flow graph.

runtime/vm/intermediate_language.cc

Index: runtime/vm/intermediate_language.cc
diff --git a/runtime/vm/intermediate_language.cc b/runtime/vm/intermediate_language.cc
index 8cc93e3e980e69d44179ac9d65a0f14058d86dec..cfa36fb2d74fedc5441679d22571d29fd62969da 100644
--- a/runtime/vm/intermediate_language.cc
+++ b/runtime/vm/intermediate_language.cc
@@ -258,6 +258,22 @@ void ThrowInstr::SetInputAt(intptr_t i, Value* value) {
 }
 
 
+intptr_t GotoInstr::InputCount() const {
+  return 0;
+}
+
+
+Value* GotoInstr::InputAt(intptr_t i) const {
+  UNREACHABLE();
+  return NULL;
+}
+
+
+void GotoInstr::SetInputAt(intptr_t i, Value* value) {
+  UNREACHABLE();
+}
+
+
 intptr_t ReturnInstr::InputCount() const {
   return 1;
 }
@@ -487,7 +503,9 @@ void BlockEntryInstr::DiscoverBlocks(
            !next_instr->IsBranch()) {
       if (vars != NULL) next_instr->RecordAssignedVars(vars);
       set_last_instruction(next_instr);
-      next_instr = next_instr->next();
+      GotoInstr* goto_instr = next_instr->AsGoto();
+      next_instr =
+          (goto_instr != NULL) ? goto_instr->successor() : next_instr->next();
     }
   }
   if (next_instr != NULL) {
@@ -539,13 +557,15 @@ void JoinEntryInstr::InsertPhi(intptr_t var_index, intptr_t var_count) {
 
 
 intptr_t Instruction::SuccessorCount() const {
-  ASSERT(next() == NULL || next()->IsBlockEntry());
-  return (next() != NULL) ? 1 : 0;
+  return 0;
 }
 
 
 BlockEntryInstr* Instruction::SuccessorAt(intptr_t index) const {
-  return next()->AsBlockEntry();
+  // Called only if index is in range.  Only control-transfer instructions
+  // can have non-zero successor counts and they override this function.
+  UNREACHABLE();
+  return NULL;
 }
 
 
@@ -573,6 +593,22 @@ BlockEntryInstr* BranchInstr::SuccessorAt(intptr_t index) const {
 }
 
 
+intptr_t GotoInstr::SuccessorCount() const {
+  return 1;
+}
+
+
+BlockEntryInstr* GotoInstr::SuccessorAt(intptr_t index) const {
+  ASSERT(index == 0);
+  return successor();
+}
+
+
+void Instruction::Goto(JoinEntryInstr* entry) {

[srdjan, 2012/07/12 16:42:26]

IMO, renaming it to AppendGoto will make the call sites more readable.
Can a Goto be appended to any instruction, i..e, do you need asserts to restrict
appending?

[Kevin Millikin (Google), 2012/07/13 15:34:59]

It's kind of messy right now.  There are asserts in set_next that cover the
disallowed cases.

+  set_next(new GotoInstr(entry));
+}
+
+
 // ==== Support for propagating static type.
 RawAbstractType* ConstantVal::StaticType() const {
   if (value().IsInstance()) {
@@ -987,6 +1023,21 @@ void ReThrowInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
 }
 
 
+LocationSummary* GotoInstr::MakeLocationSummary() const {
+  return new LocationSummary(0, 0);
+}
+
+
+void GotoInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
+  compiler->frame_register_allocator()->Spill();

[srdjan, 2012/07/12 16:42:26]

It looks strange to do spilling inside EmitNativeCode. Shouldn't this be handled
by register allocator?

[Vyacheslav Egorov (Google), 2012/07/13 14:41:42]

I agree with Srdjan. This way we have discrepancy between the way we handle
Branch (in the register allocator) and Goto (here).

[Kevin Millikin (Google), 2012/07/13 15:34:59]

I think so.  Right now, it's hardcoded into the compiler.  I just moved this
from the driver loop to the native code for Goto (since we didn't have goto
before).

Leaving it in the driver loop requires an explicit check for IsGoto(), which
seems like it should get rolled into one of the virtual functions on the
instruction.

It seems redundant to me anyway, since we also spill at basic block entries. 
I'll sort that out in another change.

[Kevin Millikin (Google), 2012/07/13 15:41:32]

I agree with you both that this is fishy.  It was already that way and I just
moved it around.

It was previously hardcoded in the compiler's VisitBlocks loop, at the end of
every basic block that ended with Goto just before the (hardcoded) jmp.

I can move it back there, or leave it here.

[srdjan, 2012/07/13 23:18:33]

I am fine with a solution that you and Slava agree with.

+  // We can fall through if the successor is the next block in the list.
+  // Otherwise, we need a jump.
+  if (!compiler->IsNextBlock(successor())) {
+    __ jmp(compiler->GetBlockLabel(successor()));
+  }
+}
+
+
 LocationSummary* BranchInstr::MakeLocationSummary() const {
   if (is_fused_with_comparison()) {
     return fused_with_comparison_->locs();