Skip to content

Commit

Permalink
Remove redundant instr merging
Browse files Browse the repository at this point in the history 
This doesn't work reliably, and we really should just lower to SSA at
some point and implement things properly.
  • Loading branch information
@yorickpeterse
yorickpeterse committed Oct 7, 2024
1 parent 51a9b1d commit 4195bb1
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 133 deletions.
7 changes: 2 additions & 5 deletions compiler/src/compiler.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -534,11 +534,8 @@ LLVM module timings:
// reduce the amount of LLVM IR we need to generate.
mir.simplify_graph();

mir.merge_redundant_instructions();

// Inlining and copy propagation may result in many redundant
// instructions being left behind, so let's try to remove some of
// those.
// Inlining and other optimizations may result in unused
// instructions, so let's get rid of those.
mir.remove_unused_instructions();
}

Expand Down
128 changes: 0 additions & 128 deletions compiler/src/mir/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1512,52 +1512,6 @@ impl Instruction {
}
}
}

fn set_target_register(&mut self, register: RegisterId) {
match self {
Instruction::Allocate(i) => i.register = register,
Instruction::Branch(_) => {}
Instruction::CallBuiltin(i) => i.register = register,
Instruction::CallClosure(i) => i.register = register,
Instruction::CallDropper(i) => i.register = register,
Instruction::CallDynamic(i) => i.register = register,
Instruction::CallExtern(i) => i.register = register,
Instruction::CallInstance(i) => i.register = register,
Instruction::CallStatic(i) => i.register = register,
Instruction::Cast(i) => i.register = register,
Instruction::CheckRefs(_) => {}
Instruction::Decrement(_) => {}
Instruction::DecrementAtomic(_) => {}
Instruction::Drop(_) => {}
Instruction::False(i) => i.register = register,
Instruction::FieldPointer(i) => i.register = register,
Instruction::Finish(_) => {}
Instruction::Float(i) => i.register = register,
Instruction::Free(_) => {}
Instruction::GetConstant(i) => i.register = register,
Instruction::GetField(i) => i.register = register,
Instruction::Goto(_) => {}
Instruction::Increment(_) => {}
Instruction::IncrementAtomic(_) => {}
Instruction::Int(i) => i.register = register,
Instruction::MethodPointer(i) => i.register = register,
Instruction::MoveRegister(i) => i.target = register,
Instruction::Nil(i) => i.register = register,
Instruction::Pointer(i) => i.register = register,
Instruction::Preempt(_) => {}
Instruction::ReadPointer(i) => i.register = register,
Instruction::Reference(i) => i.register = register,
Instruction::Return(i) => i.register = register,
Instruction::Send(_) => {}
Instruction::SetField(_) => {}
Instruction::SizeOf(i) => i.register = register,
Instruction::Spawn(i) => i.register = register,
Instruction::String(i) => i.register = register,
Instruction::Switch(_) => {}
Instruction::True(i) => i.register = register,
Instruction::WritePointer(_) => {}
}
}
}

pub(crate) struct Class {
Expand Down Expand Up @@ -2257,88 +2211,6 @@ impl Mir {
self.remove_unreachable_blocks();
}

/// Merges separate redundant instructions together into a single
/// instruction.
///
/// This is a limited form of copy propagation focused on literals and
/// chains of `Move` instructions.
pub(crate) fn merge_redundant_instructions(&mut self) {
for method in self.methods.values_mut() {
let mut defs = HashMap::new();
let uses = method.register_use_counts();

// Since we can't both mutably and immutably borrow different values
// in a Vec, we have to iterate using block and instruction indexes.
// This may incur some extra bounds checking, but sadly there's no
// way around this.
for blk_idx in 0..method.body.blocks.len() {
for ins_idx in 0..method.body.blocks[blk_idx].instructions.len()
{
let (target, source) = match &method.body.blocks[blk_idx]
.instructions[ins_idx]
{
Instruction::Int(i) => {
defs.insert(i.register, (blk_idx, ins_idx));
continue;
}
Instruction::Float(i) => {
defs.insert(i.register, (blk_idx, ins_idx));
continue;
}
Instruction::True(i) => {
defs.insert(i.register, (blk_idx, ins_idx));
continue;
}
Instruction::False(i) => {
defs.insert(i.register, (blk_idx, ins_idx));
continue;
}
Instruction::String(i) => {
defs.insert(i.register, (blk_idx, ins_idx));
continue;
}
Instruction::Nil(i) => {
defs.insert(i.register, (blk_idx, ins_idx));
continue;
}
Instruction::MoveRegister(i) if i.volatile => continue,
Instruction::MoveRegister(i) => {
// A common pattern is the following:
//
// r0 = int 123
// r1 = move r0
// r2 = move r1
//
// This ensures that if the target register is only
// used once, we collapse that (recursively) down
// into this:
//
// r2 = int 123
if uses[i.target.0] == 1 {
defs.insert(i.target, (blk_idx, ins_idx));
}

(i.target, i.source)
}
_ => continue,
};

if let Some(&(src_blk_idx, src_ins_idx)) = defs.get(&source)
{
let mut new_ins = method.body.blocks[src_blk_idx]
.instructions[src_ins_idx]
.clone();

defs.insert(target, (blk_idx, ins_idx));
new_ins.set_target_register(target);
method.body.blocks[blk_idx].instructions[ins_idx] =
new_ins;
}
}
}
}
}

/// Removes instructions that write to an unused register without side
/// effects.
///
Expand Down

0 comments on commit 4195bb1

Please sign in to comment.