Skip to content

Commit 67b3a7f

Browse files
committed
Add some nice optimizations
1 parent eb6cb7a commit 67b3a7f

1 file changed

Lines changed: 41 additions & 20 deletions

File tree

zjit/src/hir.rs

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5853,6 +5853,14 @@ impl Function {
58535853
BranchEdge { target: edge.target, args }
58545854
}
58555855

5856+
fn insn_passes_params(insn: Insn) -> bool {
5857+
match insn {
5858+
Insn::CondBranch {if_true, if_false, ..} => !if_true.args.is_empty() || !if_false.args.is_empty(),
5859+
Insn::Jump(edge) => !edge.args.is_empty(),
5860+
_ => false
5861+
}
5862+
}
5863+
58565864
// Instantiate the domain for abstract interpretation
58575865
// Outer index is block
58585866
// Inner index is param index
@@ -5861,14 +5869,11 @@ impl Function {
58615869

58625870
let mut updated = true;
58635871

5864-
// Store references to all the conditional instructions.
5865-
// We need to update these conditionals in the case of trivial block params.
5866-
let terminators: Vec<(BlockId, usize)> = self.reverse_post_order().into_iter()
5867-
.filter(|&block_id| matches!(
5868-
self.find(*self.blocks[block_id.0].insns.last().unwrap()),
5869-
Insn::CondBranch {..} | Insn::Jump {..}
5870-
))
5871-
.map(|block_id| (block_id, self.blocks[block_id.0].insns.len() - 1))
5872+
// Find blocks that terminate with Jump or CondBranch instructions that pass block params along.
5873+
// These terminators are later analyzed for trivial block params.
5874+
let param_passing_blocks: Vec<BlockId> = self.reverse_post_order().into_iter()
5875+
.filter(|&block_id|
5876+
insn_passes_params(self.find(*self.blocks[block_id.0].insns.last().unwrap())))
58725877
.collect();
58735878

58745879
while updated {
@@ -5881,18 +5886,25 @@ impl Function {
58815886
// TODO: Maybe move this outside the loop somehow? probably can't immediately, but we could keep track of a worklist of edges that change maybe?
58825887
// And only use the changed ones like a worklist? And then instead of looping to fixpoint we use a worklist based approach?
58835888
//
5884-
// Scan through each jump, collecting edges from CondBranch and Jump insns.
5885-
for (block_id, insn_index) in &terminators {
5886-
let insn_id = self.blocks[block_id.0].insns[*insn_index];
5889+
// Scan through each jump, collecting edges with params to analyze from CondBranch and Jump insns.
5890+
for block_id in &param_passing_blocks {
5891+
let insn_index = self.blocks[block_id.0].insns.len() - 1;
5892+
let insn_id = self.blocks[block_id.0].insns[insn_index];
58875893
let mut edges: Vec<BranchEdge> = vec![];
58885894

58895895
match self.find(insn_id) {
58905896
Insn::CondBranch { if_true, if_false, .. } => {
5891-
edges.push(if_true);
5892-
edges.push(if_false);
5897+
if if_true.args.len() > 0 {
5898+
edges.push(if_true);
5899+
}
5900+
if if_false.args.len() > 0 {
5901+
edges.push(if_false);
5902+
}
58935903
}
58945904
Insn::Jump(edge) => {
5895-
edges.push(edge);
5905+
if edge.args.len() > 0 {
5906+
edges.push(edge);
5907+
}
58965908
}
58975909
_ => {}
58985910
}
@@ -5927,18 +5939,26 @@ impl Function {
59275939
// 1. Replace uses of the trivial params with the concretized value
59285940
// 2. Remove trivial params from the basic block definition
59295941
// 3. Remove trivial params from each CondBranch and Jump that targets the basic block that was just updated
5930-
for (block_id, block) in predecessor_domain.iter().enumerate() {
5931-
// TODO: Don't do this
5942+
for (block_id, block_preds) in predecessor_domain.iter().enumerate() {
5943+
// If there are no block params, there is nothing to optimize
5944+
// TODO: We scan predecessors and only keep track of blocks that pass params.
5945+
// We don't do this for block_preds but we should. This requires it kind of becoming hash-mappy again :|
5946+
// This conditional is a stop-gap to get most of the gains from such an optimization, though it should be removed
5947+
// Maybe we can get around this easier by not iterating over the predecessor domain, but over a subset of indices we care about
5948+
if block_preds.len() == 0 {
5949+
continue
5950+
}
5951+
59325952
let block_id = BlockId(block_id);
59335953

5934-
let trivial_indices: Vec<usize> = block.iter().enumerate()
5954+
let trivial_indices: Vec<usize> = block_preds.iter().enumerate()
59355955
.filter_map(|(idx, state)|
59365956
matches!(state, ParamValue::One(_)).then_some(idx)
59375957
).collect();
59385958

59395959
// Replace uses of the trivial params with the concretized value
59405960
for param_index in &trivial_indices {
5941-
if let ParamValue::One(insn_id) = block[*param_index] {
5961+
if let ParamValue::One(insn_id) = block_preds[*param_index] {
59425962
self.make_equal_to(self.blocks[block_id.0].params[*param_index], insn_id);
59435963
updated = true;
59445964
}
@@ -5948,8 +5968,9 @@ impl Function {
59485968
prune_vec_by_indices(&mut self.blocks[block_id.0].params, &trivial_indices);
59495969

59505970
// Update the terminators (basic blocks can only branch at the terminator. This is where block params are passed)
5951-
for (jump_block_id, index) in &terminators {
5952-
let cond_insn_id = self.blocks[jump_block_id.0].insns[*index];
5971+
for jump_block_id in &param_passing_blocks {
5972+
let index = self.blocks[jump_block_id.0].insns.len() - 1;
5973+
let cond_insn_id = self.blocks[jump_block_id.0].insns[index];
59535974
match self.find(cond_insn_id) {
59545975
Insn::Jump(edge) => {
59555976
if edge.target == block_id {

0 commit comments

Comments
 (0)