Fix JIT codegen bug. Fixes #80.

This changes how the JIT follows which opcode to generate code for.
It's now done by following the control flow with memoization, which
makes sure that the JIT never has to reason about dead code.

vm/instructions_util.hpp

@@ -27,33 +27,41 @@ namespace rubinius {
       void at_ip(int ip) { }
       bool before(opcode op, opcode arg1 = 0, opcode arg2 = 0) { return true; }
 
-      void drive(opcode* stream, int size, int start = 0) {
-        int ip = start;
-        while(ip < size) {
-          SPECIFIC->at_ip(ip);
+      int dispatch(opcode* stream, int ip) {
+        SPECIFIC->at_ip(ip);
 
-          switch(stream[ip]) {
+        switch(stream[ip]) {
 #define HANDLE_INST0(code, name) \
-          case code: \
-                     if(SPECIFIC->before(stream[ip])) { \
-                     SPECIFIC->visit_ ## name(); } ip += 1; break;
+        case code: \
+                   if(SPECIFIC->before(stream[ip])) { \
+                     SPECIFIC->visit_ ## name(); } return ip + 1;
 
 #define HANDLE_INST1(code, name) \
-          case code: \
-                     if(SPECIFIC->before(stream[ip], stream[ip + 1])) { \
-                     SPECIFIC->visit_ ## name(stream[ip + 1]); } ip += 2; break;
+        case code: \
+                   if(SPECIFIC->before(stream[ip], stream[ip + 1])) { \
+                     SPECIFIC->visit_ ## name(stream[ip + 1]); } return ip + 2;
 
 #define HANDLE_INST2(code, name) \
-          case code: \
-                     if(SPECIFIC->before(stream[ip], stream[ip + 1], stream[ip + 2])) { \
-                     SPECIFIC->visit_ ## name(stream[ip + 1], stream[ip + 2]); } ip += 3; break;
+        case code: \
+                   if(SPECIFIC->before(stream[ip], stream[ip + 1], stream[ip + 2])) { \
+                     SPECIFIC->visit_ ## name(stream[ip + 1], stream[ip + 2]); } return ip + 3;
 
 #include "vm/gen/instruction_visitors.hpp"
 
 #undef HANDLE_INST0
 #undef HANDLE_INST1
 #undef HANDLE_INST2
-          }
+
+        default:
+          abort();
+          return -1;
+        }
+      }
+
+      void drive(opcode* stream, int size, int start = 0) {
+        int ip = start;
+        while(ip < size) {
+          ip = dispatch(stream, ip);
         }
       }
 

vm/llvm/control_flow.hpp

@@ -0,0 +1,67 @@
+#include "llvm/opcode_iter.hpp"
+
+#include <list>
+#include <map>
+
+namespace rubinius {
+  namespace jit {
+    class ControlFlowWalker {
+    public:
+      typedef std::list<int> SectionList;
+
+    private:
+      VMMethod* vmm_;
+      uint8_t* seen_;
+      SectionList work_list_;
+
+    public:
+      ControlFlowWalker(VMMethod* vmm)
+        : vmm_(vmm)
+      {
+        seen_ = new uint8_t[vmm->total];
+        memset(seen_, 0, vmm->total);
+      }
+
+      ~ControlFlowWalker() {
+        delete seen_;
+      }
+
+      void add_section(int ip) {
+        if(seen_[ip]) return;
+        work_list_.push_back(ip);
+      }
+
+      template <class EI>
+      void run(EI& each) {
+        work_list_.push_back(0);
+
+        OpcodeIterator iter(vmm_);
+
+        while(!work_list_.empty()) {
+          int ip = work_list_.back();
+          work_list_.pop_back();
+
+          iter.switch_to(ip);
+
+          while(seen_[iter.ip()] == 0) {
+            seen_[iter.ip()] = 1;
+
+            each.call(iter);
+
+            if(iter.goto_p()) {
+              opcode target = iter.goto_target();
+              assert(target >= 0 && target < vmm_->total);
+
+              add_section(target);
+            }
+
+            if(iter.terminator_p()) break;
+
+            if(!iter.next_p()) break;
+            iter.advance();
+          }
+        }
+      }
+    };
+  }
+}

vm/llvm/jit_visit.hpp

@@ -2162,7 +2162,9 @@ namespace rubinius {
     }
 
     void visit_goto(opcode ip) {
-      b().CreateBr(block_map_[ip].block);
+      BasicBlock* bb = block_map_[ip].block;
+      assert(bb);
+      b().CreateBr(bb);
       set_block(new_block("continue"));
     }
 
@@ -2178,7 +2180,9 @@ namespace rubinius {
           ConstantInt::get(ls_->IntPtrTy, cFalse), "is_true");
 
       BasicBlock* cont = new_block("continue");
-      b().CreateCondBr(cmp, block_map_[ip].block, cont);
+      BasicBlock* bb = block_map_[ip].block;
+      assert(bb);
+      b().CreateCondBr(cmp, bb, cont);
 
       set_block(cont);
     }
@@ -2195,7 +2199,9 @@ namespace rubinius {
           ConstantInt::get(ls_->IntPtrTy, cFalse), "is_true");
 
       BasicBlock* cont = new_block("continue");
-      b().CreateCondBr(cmp, block_map_[ip].block, cont);
+      BasicBlock* bb = block_map_[ip].block;
+      assert(bb);
+      b().CreateCondBr(cmp, bb, cont);
 
       set_block(cont);
     }

vm/llvm/jit_workhorse.cpp

@@ -6,6 +6,7 @@
 #include "vmmethod.hpp"
 
 #include "llvm/jit_visit.hpp"
+#include "llvm/control_flow.hpp"
 
 #include <llvm/Analysis/CaptureTracking.h>
 
@@ -1247,6 +1248,28 @@ namespace rubinius {
     loops_ = finder.loops_p();
   }
 
+  class Walker {
+    JITVisit& v_;
+    BlockMap& map_;
+
+  public:
+    Walker(JITVisit& v, BlockMap& map)
+      : v_(v)
+      , map_(map)
+    {}
+
+    void call(OpcodeIterator& iter) {
+      v_.dispatch(iter.stream(), iter.ip());
+
+      if(v_.b().GetInsertBlock()->getTerminator() == NULL) {
+        BlockMap::iterator i = map_.find(iter.next_ip());
+        if(i != map_.end()) {
+          v_.b().CreateBr(i->second.block);
+        }
+      }
+    }
+  };
+
   bool LLVMWorkHorse::generate_body() {
     JITVisit visitor(ls_, info_, block_map_, b().GetInsertBlock());
 
@@ -1264,46 +1287,13 @@ namespace rubinius {
 
     visitor.initialize();
 
-    // std::cout << info.vmm << " start: " << info.vmm->total << "\n";
-
-    // Fix up the JITBasicBlock's so that the ranges don't overlap
-    // (overlap is caused by a backward branch)
-    JITBasicBlock* prev = 0;
-    BlockMap& bm = visitor.block_map();
-    for(BlockMap::iterator i = bm.begin();
-          i != bm.end();
-          i++) {
-      JITBasicBlock& jbb = i->second;
-      if(prev && prev->end_ip >= jbb.start_ip) {
-        /*
-        std::cout << info.vmm << " overlap: "
-          << prev->start_ip << "-" << prev->end_ip << " "
-          << jbb.start_ip << "-" << jbb.end_ip
-          << "\n";
-        */
-        prev->end_ip = jbb.start_ip - 1;
-
-        /*
-        std::cout << info.vmm << " split region: " << prev->end_ip << "\n";
-        */
-      }
-
-      prev = &jbb;
-    }
-
     // Pass 2, compile!
+    // Drive by following the control flow.
+    jit::ControlFlowWalker walker(info_.vmm);
+    Walker cb(visitor, block_map_);
+
     try {
-      // Drive visitor for only blocks, as they contain all live regions
-      for(BlockMap::iterator i = bm.begin();
-          i != bm.end();
-          i++) {
-        JITBasicBlock& jbb = i->second;
-        /*
-        std::cout << info.vmm
-          << " Driving: " << jbb.start_ip << "-" << jbb.end_ip << "\n";
-        */
-        visitor.drive(info_.vmm->opcodes, jbb.end_ip+1, jbb.start_ip);
-      }
+      walker.run<Walker>(cb);
     } catch(JITVisit::Unsupported &e) {
       return false;
     }

vm/llvm/opcode_iter.hpp

@@ -0,0 +1,128 @@
+namespace rubinius {
+  class OpcodeIterator {
+    VMMethod* vmm_;
+
+    opcode ip_;
+    int width_;
+
+    opcode op_;
+    const char* name_;
+
+  private:
+    void update() {
+      op_ = vmm_->opcodes[ip_];
+
+      switch(op_) {
+#define HANDLE_INST0(code, name) \
+      case code: \
+        name_ = #name; \
+        width_ = 1; \
+        break;
+#define HANDLE_INST1(code, name) \
+      case code: \
+        name_ = #name; \
+        width_ = 2; \
+        break;
+#define HANDLE_INST2(code, name) \
+      case code: \
+        name_ = #name; \
+        width_ = 3; \
+        break;
+#include "gen/instruction_visitors.hpp"
+#undef HANDLE_INST0
+#undef HANDLE_INST1
+#undef HANDLE_INST2
+
+      default:
+        abort();
+      }
+    }
+
+
+#include "gen/instruction_names.hpp"
+
+  public:
+    OpcodeIterator(VMMethod* vmm, int ip=0)
+      : vmm_(vmm)
+      , ip_(ip)
+      , width_(0)
+    {
+      update();
+    }
+
+    opcode* stream() {
+      return vmm_->opcodes;
+    }
+
+    opcode ip() {
+      return ip_;
+    }
+
+    int next_ip() {
+      return ip_ + width_;
+    }
+
+    bool next_p() {
+      return ip_ < vmm_->total;
+    }
+
+    void switch_to(int ip) {
+      ip_ = ip;
+      update();
+    }
+
+    void advance() {
+      switch_to(next_ip());
+    }
+
+    opcode op() {
+      return op_;
+    }
+
+    const char* name() {
+      return name_;
+    }
+
+    int operands() {
+      return width_ - 1;
+    }
+
+    opcode operand(int which) {
+      return vmm_->opcodes[ip_ + which + 1];
+    }
+
+    // flags
+    bool terminator_p() {
+      switch(op_) {
+      case insn_ret:
+      case insn_goto:
+      case insn_raise_exc:
+      case insn_raise_return:
+      case insn_raise_break:
+      case insn_ensure_return:
+      case insn_reraise:
+        return true;
+      default:
+        return false;
+      }
+    }
+
+    bool goto_p() {
+      switch(op_) {
+      case insn_goto:
+      case insn_goto_if_true:
+      case insn_goto_if_false:
+      case insn_setup_unwind:
+        return true;
+      default:
+        return false;
+      }
+    }
+
+    opcode goto_target() {
+      // All gotos use operand 0 as the target
+      return operand(0);
+    }
+
+  };
+}