Merge pull request #1399 from undingen/bst_inplace2

reduce the size of CFGBlocks

src/analysis/fpc.h

@@ -83,8 +83,7 @@ void computeFixedPoint(typename BBAnalyzer<T>::Map&& initial_map, CFGBlock* init
 
         analyzer.processBB(ending, block);
 
-        for (int i = 0; i < block->successors.size(); i++) {
-            CFGBlock* next_block = block->successors[i];
+        for (CFGBlock* next_block : block->successors()) {
             bool changed = false;
             bool initial = false;
             if (starting_states.count(next_block) == 0) {

src/analysis/function_analysis.cpp

@@ -125,7 +125,7 @@ bool LivenessAnalysis::isLiveAtEnd(int vreg, CFGBlock* block) {
         return false;
 #endif
 
-    if (block->successors.size() == 0)
+    if (block->successors().size() == 0)
         return false;
 
     if (!result_cache[vreg].size()) {
@@ -376,10 +376,11 @@ PhiAnalysis::PhiAnalysis(VRegMap<DefinednessAnalysis::DefinitionLevel> initial_m
 }
 
 const VRegSet& PhiAnalysis::getAllRequiredAfter(CFGBlock* block) {
-    if (block->successors.size() == 0)
+    auto successors = block->successors();
+    if (successors.size() == 0)
         return empty_set;
-    assert(required_phis.count(block->successors[0]));
-    return required_phis.find(block->successors[0])->second;
+    assert(required_phis.count(successors[0]));
+    return required_phis.find(successors[0])->second;
 }
 
 const VRegSet& PhiAnalysis::getAllRequiredFor(CFGBlock* block) {
@@ -394,19 +395,20 @@ bool PhiAnalysis::isRequired(int vreg, CFGBlock* block) {
 }
 
 bool PhiAnalysis::isRequiredAfter(int vreg, CFGBlock* block) {
+    auto successors = block->successors();
     assert(vreg >= 0);
     // If there are multiple successors, then none of them are allowed
     // to require any phi nodes
-    if (block->successors.size() != 1)
+    if (successors.size() != 1)
         return false;
 
     // Fall back to the other method:
-    return isRequired(vreg, block->successors[0]);
+    return isRequired(vreg, successors[0]);
 }
 
 bool PhiAnalysis::isPotentiallyUndefinedAfter(int vreg, CFGBlock* block) {
     assert(vreg >= 0);
-    for (auto b : block->successors) {
+    for (auto b : block->successors()) {
         if (isPotentiallyUndefinedAt(vreg, b))
             return true;
     }

src/analysis/type_analysis.cpp

@@ -51,8 +51,9 @@ ConcreteCompilerType* NullTypeAnalysis::getTypeAtBlockStart(int vreg, CFGBlock*
 }
 
 ConcreteCompilerType* NullTypeAnalysis::getTypeAtBlockEnd(int vreg, CFGBlock* block) {
-    assert(block->successors.size() > 0);
-    return getTypeAtBlockStart(vreg, block->successors[0]);
+    auto successors = block->successors();
+    assert(successors.size() > 0);
+    return getTypeAtBlockStart(vreg, successors[0]);
 }
 
 
@@ -571,8 +572,9 @@ private:
 
 public:
     ConcreteCompilerType* getTypeAtBlockEnd(int vreg, CFGBlock* block) override {
-        assert(block->successors.size() > 0);
-        return getTypeAtBlockStart(vreg, block->successors[0]);
+        auto successors = block->successors();
+        assert(successors.size() > 0);
+        return getTypeAtBlockStart(vreg, successors[0]);
     }
     ConcreteCompilerType* getTypeAtBlockStart(int vreg, CFGBlock* block) override {
         assert(starting_types.count(block));
@@ -686,8 +688,7 @@ public:
                 }
             }
 
-            for (int i = 0; i < block->successors.size(); i++) {
-                CFGBlock* next_block = block->successors[i];
+            for (CFGBlock* next_block : block->successors()) {
                 bool first = (starting_types.count(next_block) == 0);
                 if (first)
                     starting_types.insert(std::make_pair(next_block, TypeMap(num_vregs)));

src/codegen/irgen.cpp

@@ -246,8 +246,7 @@ static std::vector<std::pair<CFGBlock*, CFGBlock*>> computeBlockTraversalOrder(c
         while (idx < rtn.size()) {
             CFGBlock* cur = rtn[idx].first;
 
-            for (int i = 0; i < cur->successors.size(); i++) {
-                CFGBlock* b = cur->successors[i];
+            for (CFGBlock* b : cur->successors()) {
                 assert(blocks.count(b));
                 if (in_queue.count(b))
                     continue;
@@ -268,7 +267,7 @@ static std::vector<std::pair<CFGBlock*, CFGBlock*>> computeBlockTraversalOrder(c
                 continue;
 
             // Avoid picking any blocks where we can't add an epilogue to the predecessors
-            if (b->predecessors.size() == 1 && b->predecessors[0]->successors.size() > 1)
+            if (b->predecessors.size() == 1 && b->predecessors[0]->successors().size() > 1)
                 continue;
 
             if (best == NULL || b->idx < best->idx)
@@ -742,7 +741,7 @@ static void emitBBs(IRGenState* irstate, TypeAnalysis* types, const OSREntryDesc
                 //   are disallowed
 
                 auto pred = block->predecessors[0];
-                auto last_inst = pred->getLastStmt();
+                auto last_inst = pred->getTerminator();
 
                 SymbolTable* sym_table = ending_symbol_tables[pred];
                 bool created_new_sym_table = false;
@@ -824,7 +823,7 @@ static void emitBBs(IRGenState* irstate, TypeAnalysis* types, const OSREntryDesc
         llvm_exit_blocks[block] = ending_st.ending_block;
 
         if (ending_st.exception_state.size()) {
-            BST_stmt* last_stmt = block->getLastStmt();
+            BST_stmt* last_stmt = block->getTerminator();
             assert(last_stmt->is_invoke());
             CFGBlock* exc_block = last_stmt->get_exc_block();
             assert(!incoming_exception_state.count(exc_block));
@@ -972,8 +971,7 @@ static void computeBlockSetClosure(BlockSet& blocks) {
             continue;
         expanded.insert(b);
 
-        for (int i = 0; i < b->successors.size(); i++) {
-            CFGBlock* b2 = b->successors[i];
+        for (CFGBlock* b2 : b->successors()) {
             blocks.insert(b2);
             q.push_back(b2);
         }

src/codegen/irgen/irgenerator.cpp

@@ -2604,7 +2604,7 @@ private:
                 ConcreteCompilerVariable* v = val->makeConverted(emitter, phi_type);
                 symbol_table[vreg] = v;
             } else {
-                if (myblock->successors.size()) {
+                if (myblock->successors().size()) {
                     // TODO getTypeAtBlockEnd will automatically convert up to the concrete type, which we don't
                     // want
                     // here, but this is just for debugging so I guess let it happen for now:
@@ -2730,20 +2730,21 @@ public:
             ASSERT(p.second->getType()->isUsable(), "%d", p.first);
         }
 
-        if (myblock->successors.size() == 0) {
+        auto successors = myblock->successors();
+        if (successors.size() == 0) {
             st->clear();
             symbol_table.clear();
             return EndingState(st, phi_st, def_vars, curblock, outgoing_exc_state);
-        } else if (myblock->successors.size() > 1) {
+        } else if (successors.size() > 1) {
             // Since there are no critical edges, all successors come directly from this node,
             // so there won't be any required phis.
             return EndingState(st, phi_st, def_vars, curblock, outgoing_exc_state);
         }
 
-        assert(myblock->successors.size() == 1); // other cases should have been handled
+        assert(successors.size() == 1); // other cases should have been handled
 
         // In theory this case shouldn't be necessary:
-        if (myblock->successors[0]->predecessors.size() == 1) {
+        if (successors[0]->predecessors.size() == 1) {
             // If the next block has a single predecessor, don't have to
             // emit any phis.
             // Should probably not emit no-op jumps like this though.

src/core/cfg.cpp

@@ -231,25 +231,33 @@ static int getLastLineno(llvm::ArrayRef<AST_stmt*> body, int default_lineno) {
     return getLastLinenoSub(body.back());
 }
 
-void CFGBlock::connectTo(CFGBlock* successor, bool allow_backedge) {
-    assert(successors.size() <= 1);
+llvm::SmallVector<CFGBlock*, 2> CFGBlock::successors() const {
+    llvm::SmallVector<CFGBlock*, 2> successors;
+    auto* last = getTerminator();
+    if (last->type() == BST_TYPE::Jump) {
+        successors.push_back(bst_cast<BST_Jump>(last)->target);
+    } else if (last->type() == BST_TYPE::Branch) {
+        assert(bst_cast<BST_Branch>(last)->iftrue != bst_cast<BST_Branch>(last)->iffalse);
+        successors.push_back(bst_cast<BST_Branch>(last)->iftrue);
+        successors.push_back(bst_cast<BST_Branch>(last)->iffalse);
+    } else if (last->is_invoke()) {
+        successors.push_back(last->get_normal_block());
+        if (last->get_exc_block() != last->get_normal_block())
+            successors.push_back(last->get_exc_block());
+    }
+    return successors;
+}
 
+void CFGBlock::connectTo(CFGBlock* successor, bool allow_backedge) {
     if (!allow_backedge) {
         assert(this->idx >= 0);
         ASSERT(successor->idx == -1 || successor->idx > this->idx, "edge from %d (%s) to %d (%s)", this->idx,
                this->info, successor->idx, successor->info);
     }
-    // assert(successors.count(successor) == 0);
-    // assert(successor->predecessors.count(this) == 0);
-
-    successors.push_back(successor);
     successor->predecessors.push_back(this);
 }
 
 void CFGBlock::unconnectFrom(CFGBlock* successor) {
-    // assert(successors.count(successor));
-    // assert(successor->predecessors.count(this));
-    successors.erase(std::remove(successors.begin(), successors.end(), successor), successors.end());
     successor->predecessors.erase(std::remove(successor->predecessors.begin(), successor->predecessors.end(), this),
                                   successor->predecessors.end());
 }
@@ -264,8 +272,8 @@ void CFGBlock::print(const CodeConstants& code_constants, llvm::raw_ostream& str
         stream << " " << predecessors[j]->idx;
     }
     stream << " Successors:";
-    for (int j = 0; j < successors.size(); j++) {
-        stream << " " << successors[j]->idx;
+    for (CFGBlock* successor : successors()) {
+        stream << " " << successor->idx;
     }
     stream << "\n";
 
@@ -3225,12 +3233,13 @@ static int pruneUnnecessaryBlocks(CFG* rtn) {
     for (auto it = rtn->blocks.begin(); it != rtn->blocks.end(); ++it) {
         CFGBlock* b = *it;
 
-        if (b->successors.size() == 1) {
-            CFGBlock* b2 = b->successors[0];
+        auto b_successors = b->successors();
+        if (b_successors.size() == 1) {
+            CFGBlock* b2 = b_successors[0];
             if (b2->predecessors.size() != 1)
                 continue;
 
-            auto last_stmt = b->getLastStmt();
+            auto last_stmt = b->getTerminator();
             if (last_stmt->is_invoke()) {
                 // TODO probably shouldn't be generating these anyway:
                 assert(last_stmt->get_normal_block() == last_stmt->get_exc_block());
@@ -3246,7 +3255,7 @@ static int pruneUnnecessaryBlocks(CFG* rtn) {
 
             b->unconnectFrom(b2);
 
-            for (CFGBlock* b3 : b2->successors) {
+            for (CFGBlock* b3 : b2->successors()) {
                 b->connectTo(b3, true);
                 b2->unconnectFrom(b3);
             }
@@ -3268,7 +3277,7 @@ static int pruneUnnecessaryBlocks(CFG* rtn) {
             bool should_merge_blocks = blocks_to_merge.count(b);
             if (should_merge_blocks) {
                 // copy first block without the terminator
-                block_size -= b->getLastStmt()->size_in_bytes();
+                block_size -= b->getTerminator()->size_in_bytes();
                 memcpy(final_bytecode.allocate(block_size), b->body(), block_size);
                 offset += block_size;
                 // copy second block and delete it
@@ -3394,14 +3403,14 @@ static std::pair<CFG*, CodeConstants> computeCFG(llvm::ArrayRef<AST_stmt*> body,
         for (CFGBlock* b2 : b->predecessors) {
             ASSERT(b2->idx != -1 && b->isPlaced(), "Forgot to place a block!");
         }
-        for (CFGBlock* b2 : b->successors) {
+        for (CFGBlock* b2 : b->successors()) {
             ASSERT(b2->idx != -1 && b->isPlaced(), "Forgot to place a block!");
         }
 
         ASSERT(b->body() != NULL, "%d", b->idx);
-        ASSERT(b->successors.size() <= 2, "%d has too many successors!", b->idx);
-        if (b->successors.size() == 0) {
-            BST_stmt* terminator = b->getLastStmt();
+        ASSERT(b->successors().size() <= 2, "%d has too many successors!", b->idx);
+        if (b->successors().size() == 0) {
+            BST_stmt* terminator = b->getTerminator();
             assert(terminator->type() == BST_TYPE::Return || terminator->type() == BST_TYPE::Raise
                    || terminator->type() == BST_TYPE::Assert);
         }
@@ -3420,11 +3429,11 @@ static std::pair<CFG*, CodeConstants> computeCFG(llvm::ArrayRef<AST_stmt*> body,
     // the cfg-computing code directly avoids making critical edges.
     // Either way, double check to make sure that we don't have any:
     for (int i = 0; i < rtn->blocks.size(); i++) {
-        if (rtn->blocks[i]->successors.size() >= 2) {
-            for (int j = 0; j < rtn->blocks[i]->successors.size(); j++) {
+        auto successors = rtn->blocks[i]->successors();
+        if (successors.size() >= 2) {
+            for (CFGBlock* successor : successors) {
                 // It's ok to have zero predecessors if you are the entry block
-                ASSERT(rtn->blocks[i]->successors[j]->predecessors.size() < 2, "Critical edge from %d to %d!", i,
-                       rtn->blocks[i]->successors[j]->idx);
+                ASSERT(successor->predecessors.size() < 2, "Critical edge from %d to %d!", i, successor->idx);
             }
         }
     }

src/core/cfg.h

@@ -75,9 +75,9 @@ public:
     // contains the address of the entry function
     std::pair<CFGBlock*, Box*>(*entry_code)(void* interpeter, CFGBlock* block, Box** vregs);
 
-    llvm::SmallVector<CFGBlock*, 2> predecessors, successors;
-    int idx; // index in the CFG
+    llvm::TinyPtrVector<CFGBlock*> predecessors;
     const char* info;
+    int idx;                  // index in the CFG
     int offset_of_first_stmt; // offset of this block into the bytecode array in bytes
 
 #ifndef NDEBUG
@@ -85,8 +85,11 @@ public:
     bool allowed_to_add_stuff = false;
 #endif
 
+    // returns the successors by looking at the terminator (which requires iterating over all instructions in the block)
+    llvm::SmallVector<CFGBlock*, 2> successors() const;
+
     CFGBlock(CFG* cfg, int idx, const char* info = NULL)
-        : cfg(cfg), code(NULL), entry_code(NULL), idx(idx), info(info), offset_of_first_stmt(-1) {}
+        : cfg(cfg), code(NULL), entry_code(NULL), info(info), idx(idx), offset_of_first_stmt(-1) {}
 
     BST_stmt* body() {
         auto it = begin();
@@ -99,7 +102,7 @@ public:
         }
         return size;
     }
-    BST_stmt* getLastStmt() const {
+    BST_stmt* getTerminator() const {
         // TODO: this is inefficient
         for (BST_stmt* stmt : *this) {
             if (stmt->is_terminator())

test/unittests/analysis.cpp

@@ -75,7 +75,7 @@ TEST_F(AnalysisTest, augassign) {
 
     for (CFGBlock* block : cfg->blocks) {
         //printf("%d\n", block->idx);
-        if (block->getLastStmt()->type() != BST_TYPE::Return)
+        if (block->getTerminator()->type() != BST_TYPE::Return)
             ASSERT_TRUE(liveness->isLiveAtEnd(vregs.getVReg(module->interned_strings->get("a")), block));
     }