From 6a82b7b879ef8ddaaa9bb060a750a226ef643d25 Mon Sep 17 00:00:00 2001 From: Jez Ng Date: Tue, 31 Oct 2017 01:37:32 -0700 Subject: [PATCH] regalloc: Always rebuild CFG after inserting instructions Summary: The lack of rebuilding caused a bug in the code that inserted loads for param register in their immediate dominators. Roughly, we started with this code: B0: load-param v16 load-param v17 load-param v18 load-param v19 if-eqz v16 :foo B1: if-eqz v17 :bar In the first iteration of the regalloc loop, we inserted a load for v18 in B1. Without rebuilding the CFG, we end up with: B0: load-param v16 load-param v17 load-param v18 load-param v19 if-eqz v16 :foo move v0 v18 B1: if-eqz v17 :bar Now in the second iteration of the regalloc loop, we wanted to insert a load for v19 in B0. We looked to see if the last instruction was a branch opcode -- that would necessitate inserting the move instruction before the branch -- but since the last opcode in B0 is now a move, we inserted our load in the wrong place. I'm not sure why the ART verifier doesn't choke on this, but I'm glad arnaudvenet's IRTypeChecker did... Reviewed By: kgsharma Differential Revision: D6194764 fbshipit-source-id: f7945af4771a82afc6c65f9839b84d34dfc18f15 --- opt/regalloc/GraphColoring.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/opt/regalloc/GraphColoring.cpp b/opt/regalloc/GraphColoring.cpp index 136c4edb0ea..656e39deb02 100644 --- a/opt/regalloc/GraphColoring.cpp +++ b/opt/regalloc/GraphColoring.cpp @@ -1099,10 +1099,11 @@ void Allocator::allocate(bool use_splitting, IRCode* code) { TRACE(REG, 5, "Split plan:\n%s\n", SHOW(split_plan)); m_stats.split_moves += split(fixpoint_iter, split_plan, split_costs, ig, code); - // Since in split we might have inserted new blocks to load between - // blocks. So we call build_cfg() again to have a correct cfg. - code->build_cfg(); } + + // Since we have inserted instructions, we need to rebuild the CFG to + // ensure that block boundaries remain correct + code->build_cfg(); } else { transform::remap_registers(code, reg_transform.map); code->set_registers_size(reg_transform.size);