Move BasicBlock::splitAfter to CFG

Summary: BasicBlock shouldn't be in the business of modifying the CFG.

Reviewed By: DinoV

Differential Revision: D88176338

fbshipit-source-id: ac098a197414d7b58cc347c52c01b4f1e6d0eaf3

Author: alexmalyshev

cinderx/Jit/hir/hir.cpp

@@ -658,22 +658,6 @@ bool BasicBlock::IsTrampoline() {
   return false;
 }
 
-BasicBlock* BasicBlock::splitAfter(Instr& instr) {
-  JIT_CHECK(cfg != nullptr, "cannot split unlinked block");
-  auto tail = cfg->AllocateBlock();
-  for (auto it = std::next(instrs_.iterator_to(instr)); it != instrs_.end();) {
-    auto& instr_2 = *it;
-    ++it;
-    instr_2.unlink();
-    tail->Append(&instr_2);
-  }
-
-  for (auto edge : tail->out_edges()) {
-    edge->to()->fixupPhis(this, tail);
-  }
-  return tail;
-}
-
 void BasicBlock::fixupPhis(BasicBlock* old_pred, BasicBlock* new_pred) {
   // This won't work correctly if this block has two incoming edges from the
   // same block, but we already can't handle that correctly with our current Phi
@@ -765,6 +749,22 @@ void CFG::RemoveBlock(BasicBlock* block) {
   block->cfg = nullptr;
 }
 
+BasicBlock* CFG::splitAfter(Instr& target) {
+  auto block = target.block();
+  auto tail = AllocateBlock();
+  for (auto it = std::next(block->iterator_to(target)); it != block->end();) {
+    auto& instr = *it;
+    ++it;
+    instr.unlink();
+    tail->Append(&instr);
+  }
+
+  for (auto edge : tail->out_edges()) {
+    edge->to()->fixupPhis(block, tail);
+  }
+  return tail;
+}
+
 void CFG::splitCriticalEdges() {
   std::vector<Edge*> critical_edges;
 

cinderx/Jit/hir/hir.h

@@ -4000,11 +4000,6 @@ class BasicBlock {
   explicit BasicBlock(int id_) : id(id_), cfg(nullptr) {}
   ~BasicBlock();
 
-  // Split this block after instr. Once split, this block will contain all
-  // instructions up to and including instr. A newly allocated block is returned
-  // that contains all instructions following instr.
-  BasicBlock* splitAfter(Instr& instr);
-
   // Replace any references to old_pred in this block's Phis with new_pred.
   void fixupPhis(BasicBlock* old_pred, BasicBlock* new_pred);
   // Adds a new predecessor to the phi that follows from the old predecessor
@@ -4203,6 +4198,11 @@ class CFG {
   // Remove block from the CFG
   void RemoveBlock(BasicBlock* block);
 
+  // Split a block after instr. Once split, the block will contain all
+  // instructions up to and including instr. A newly allocated block is returned
+  // that contains all instructions following instr.
+  BasicBlock* splitAfter(Instr& target);
+
   // Split any critical edges by inserting trampoline blocks.
   void splitCriticalEdges();
 

cinderx/Jit/hir/inliner.cpp

@@ -246,8 +246,7 @@ void inlineFunctionCall(Function& caller, AbstractCall* call_instr) {
       caller.fullname);
 
   BorrowedRef<PyCodeObject> callee_code{callee->func_code};
-  BasicBlock* head = call_instr->instr->block();
-  BasicBlock* tail = head->splitAfter(*call_instr->instr);
+  BasicBlock* tail = caller.cfg.splitAfter(*call_instr->instr);
   auto begin_inlined_function = BeginInlinedFunction::create(
       callee, std::move(caller_frame_state), callee_name);
   auto callee_branch = Branch::create(result.entry);

cinderx/Jit/hir/simplify.cpp

@@ -167,7 +167,7 @@ struct Env {
     JIT_CHECK(
         cursor != block->begin(),
         "block should not be empty after calling do_branch()");
-    BasicBlock* tail = block->splitAfter(*std::prev(cursor));
+    BasicBlock* tail = func.cfg.splitAfter(*std::prev(cursor));
 
     block = bb1;
     cursor = bb1->end();
@@ -212,7 +212,7 @@ struct Env {
     BasicBlock* slow_path = func.cfg.AllocateBlock();
 
     auto branch = do_branch(slow_path);
-    BasicBlock* fast_path = block->splitAfter(*branch);
+    BasicBlock* fast_path = func.cfg.splitAfter(*branch);
     branch->set_true_bb(fast_path);
 
     block = slow_path;

cinderx/RuntimeTests/hir_test.cpp

@@ -57,7 +57,7 @@ TEST(BasicBlockTest, SplitAfterSplitsBlockAfterInstruction) {
   head->append<LoadConst>(v0, TNoneType);
   Instr* load_const = head->GetTerminator();
   head->append<Return>(v0);
-  BasicBlock* tail = head->splitAfter(*load_const);
+  BasicBlock* tail = cfg.splitAfter(*load_const);
   ASSERT_NE(nullptr, head->GetTerminator());
   EXPECT_TRUE(head->GetTerminator()->IsLoadConst());
   ASSERT_NE(nullptr, tail->GetTerminator());