Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions crates/rue-codegen/src/aarch64/cfg_lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,8 +375,13 @@ impl<'a> CfgLower<'a> {
}
}

// The manifest's control contract travels with the call: a trap helper
// aborts, so liveness must not propagate anything past it (RUE-1224).
let symbol_id = self.intern_symbol(plan.symbol());
self.mir.push(Aarch64Inst::Bl { symbol_id });
self.mir.push(Aarch64Inst::Bl {
symbol_id,
returns: plan.return_behavior(),
});

match plan.result() {
RuntimeCallResult::OutPointer(shape) => {
Expand Down Expand Up @@ -880,7 +885,7 @@ impl<'a> CfgLower<'a> {
});
}
let symbol_id = self.intern_symbol(plan.target.symbol());
self.mir.push(Aarch64Inst::Bl { symbol_id });
self.mir.push(Aarch64Inst::call(symbol_id));
if plan.stack_bytes > 0 {
self.mir.push(Aarch64Inst::AddImm {
dst: Operand::Physical(Reg::Sp),
Expand Down Expand Up @@ -1153,7 +1158,7 @@ impl<'a> CfgLower<'a> {
});
}
let symbol_id = self.intern_symbol(inputs.symbol_ref());
self.mir.push(Aarch64Inst::Bl { symbol_id });
self.mir.push(Aarch64Inst::call(symbol_id));
if stack_bytes > 0 {
self.mir.push(Aarch64Inst::AddImm {
dst: Operand::Physical(Reg::Sp),
Expand Down Expand Up @@ -4263,7 +4268,7 @@ mod tests {
mir.instructions()
.iter()
.position(|inst| {
matches!(inst, Aarch64Inst::Bl { symbol_id } if mir.get_symbol(*symbol_id) == helper.symbol())
matches!(inst, Aarch64Inst::Bl { symbol_id, .. } if mir.get_symbol(*symbol_id) == helper.symbol())
})
.unwrap_or_else(|| panic!("missing call to {helper:?}"))
}
Expand Down
6 changes: 3 additions & 3 deletions crates/rue-codegen/src/aarch64/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1383,7 +1383,7 @@ impl<'a> Emitter<'a> {
self.record_label(format!("L{}", id));
}

Aarch64Inst::Bl { symbol_id } => {
Aarch64Inst::Bl { symbol_id, .. } => {
let symbol = self.mir.get_symbol(*symbol_id);
self.begin_inst();
self.emit_bl(symbol);
Expand Down Expand Up @@ -3386,7 +3386,7 @@ mod tests {

let mut mir = Aarch64Mir::new();
let symbol_id = mir.intern_symbol("test_func");
mir.push(Aarch64Inst::Bl { symbol_id });
mir.push(Aarch64Inst::call(symbol_id));

let (code, relocs) = Emitter::new(&mir, 0, 0, 0, &[], &[])
.without_frame()
Expand Down Expand Up @@ -3428,7 +3428,7 @@ mod tests {
// (RUE-1195).
let mut mir = Aarch64Mir::new();
let symbol_id = mir.intern_symbol("callee");
mir.push(Aarch64Inst::Bl { symbol_id });
mir.push(Aarch64Inst::call(symbol_id));
mir.push(Aarch64Inst::Ret);

let expected: Vec<u32> = vec![
Expand Down
15 changes: 13 additions & 2 deletions crates/rue-codegen/src/aarch64/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

use std::collections::HashMap;

use super::mir::{Aarch64Inst, Aarch64Mir, Operand, Reg};
use super::mir::{Aarch64Inst, Aarch64Mir, Operand, Reg, ReturnBehavior};
use crate::liveness::{
LivenessAdapter, branch_successor, conditional_successors, fallthrough_successor,
};
Expand Down Expand Up @@ -57,6 +57,10 @@ impl LivenessAdapter for Aarch64LivenessAdapter<'_> {
fn clobbers(&self, inst: &Self::Inst) -> Vec<Self::Reg> {
inst.clobbers().to_vec()
}

fn is_non_returning(&self, inst: &Self::Inst) -> bool {
inst.is_non_returning()
}
}

/// Compute liveness information for Aarch64Mir.
Expand Down Expand Up @@ -111,7 +115,14 @@ fn get_successors(
}
// Return and trap have no successors
Aarch64Inst::Ret | Aarch64Inst::Brk => Vec::new(),
// Function calls fall through (callee returns)
// A call to a helper the runtime ABI manifest declares
// `ReturnBehavior::Never` aborts the process. Control never comes back,
// so it has no successors and nothing is live after it (RUE-1224).
Aarch64Inst::Bl {
returns: ReturnBehavior::Never,
..
} => Vec::new(),
// Ordinary calls fall through (the callee returns)
Aarch64Inst::Bl { .. } => fallthrough_successor(idx, num_insts),
// All other instructions fall through to the next
_ => fallthrough_successor(idx, num_insts),
Expand Down
41 changes: 38 additions & 3 deletions crates/rue-codegen/src/aarch64/mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
use std::collections::HashMap;
use std::fmt;

pub use rue_runtime_abi::ReturnBehavior;

// Compile-time size assertions to prevent silent size growth during refactoring.
// These limits are set slightly above current sizes to allow minor changes,
// but will catch significant size regressions.
Expand Down Expand Up @@ -893,7 +895,16 @@ pub enum Aarch64Inst {
/// `bl symbol` - Branch with link (call).
///
/// The `symbol_id` is an index into the symbol table stored in `Aarch64Mir`.
Bl { symbol_id: u32 },
///
/// `returns` carries the callee's control contract. Every Rue-to-Rue call
/// returns; a runtime helper carries whatever the ABI manifest declares, so
/// the trap helpers (`__rue_overflow` and siblings) are `Never`. Liveness
/// gives a `Never` call no successors, and allocation does not count its
/// clobbers against a value that is only live around it (RUE-1224).
Bl {
symbol_id: u32,
returns: ReturnBehavior,
},

/// `ret` - Return (branch to LR).
Ret,
Expand Down Expand Up @@ -939,6 +950,30 @@ pub enum Aarch64Inst {
}

impl Aarch64Inst {
/// A call to a callee that returns normally.
///
/// Every Rue-to-Rue call and every returning runtime helper uses this. A
/// call to a helper the ABI manifest declares `ReturnBehavior::Never` must
/// build [`Aarch64Inst::Bl`] directly with the manifest's behavior so
/// liveness sees it (RUE-1224).
pub const fn call(symbol_id: u32) -> Self {
Self::Bl {
symbol_id,
returns: ReturnBehavior::Returns,
}
}

/// Whether this instruction never returns control to the next one.
pub const fn is_non_returning(&self) -> bool {
matches!(
self,
Aarch64Inst::Bl {
returns: ReturnBehavior::Never,
..
}
)
}

/// Returns physical registers clobbered by this instruction.
///
/// This information is used by the register allocator to avoid assigning
Expand Down Expand Up @@ -1252,7 +1287,7 @@ impl fmt::Display for Aarch64Inst {
Aarch64Inst::Bvs { label } => write!(f, "b.vs {}", label),
Aarch64Inst::Bvc { label } => write!(f, "b.vc {}", label),
Aarch64Inst::Label { id } => write!(f, "{}:", id),
Aarch64Inst::Bl { symbol_id } => write!(f, "bl sym{}", symbol_id),
Aarch64Inst::Bl { symbol_id, .. } => write!(f, "bl sym{}", symbol_id),
Aarch64Inst::Ret => write!(f, "ret"),
Aarch64Inst::Brk => write!(f, "brk #0x1"),
Aarch64Inst::Svc { imm } => write!(f, "svc #{:#x}", imm),
Expand Down Expand Up @@ -1456,7 +1491,7 @@ impl fmt::Display for Aarch64Mir {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
for inst in &self.instructions {
// Special handling for Bl to show actual symbol name
if let Aarch64Inst::Bl { symbol_id } = inst {
if let Aarch64Inst::Bl { symbol_id, .. } = inst {
writeln!(f, " bl {}", self.get_symbol(*symbol_id))?;
} else {
writeln!(f, " {}", inst)?;
Expand Down
113 changes: 107 additions & 6 deletions crates/rue-codegen/src/aarch64/regalloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1050,7 +1050,9 @@ impl RegAlloc {
Aarch64Inst::Bvs { label } => mir.push(Aarch64Inst::Bvs { label }),
Aarch64Inst::Bvc { label } => mir.push(Aarch64Inst::Bvc { label }),
Aarch64Inst::Label { id } => mir.push(Aarch64Inst::Label { id }),
Aarch64Inst::Bl { symbol_id } => mir.push(Aarch64Inst::Bl { symbol_id }),
Aarch64Inst::Bl { symbol_id, returns } => {
mir.push(Aarch64Inst::Bl { symbol_id, returns })
}
Aarch64Inst::Ret => mir.push(Aarch64Inst::Ret),
Aarch64Inst::Brk => mir.push(Aarch64Inst::Brk),
Aarch64Inst::Svc { imm } => mir.push(Aarch64Inst::Svc { imm }),
Expand Down Expand Up @@ -1353,6 +1355,12 @@ impl RegAllocBackend for Aarch64Backend {
}
}

fn physical_operands(inst: &Self::Inst) -> Vec<Self::Reg> {
let mut regs = super::schedule::regs_read(inst);
regs.extend(super::schedule::regs_written(inst));
regs
}

fn new_mir() -> Self::Mir {
Aarch64Mir::new()
}
Expand Down Expand Up @@ -1944,7 +1952,7 @@ mod tests {
.collect();

define_loaded_values(&mut mir, &vregs);
mir.push(Aarch64Inst::Bl { symbol_id: symbol });
mir.push(Aarch64Inst::call(symbol));
for &vreg in &vregs {
mir.push(Aarch64Inst::MovRR {
dst: Operand::Physical(Reg::X0),
Expand Down Expand Up @@ -1976,6 +1984,99 @@ mod tests {
}
}

#[test]
fn trap_crossing_values_still_take_caller_saved_registers() {
// The same fixture as `cross_call_values_never_take_a_caller_saved_register`,
// except the call is the overflow trap. It never returns, so no value
// is live across it and the caller-saved class stays available
// (RUE-1224).
let mut mir = Aarch64Mir::new();
let symbol = mir.intern_symbol(rue_runtime_abi::RuntimeHelperId::Overflow.symbol());
let vregs: Vec<VReg> = (0..ALLOCATABLE_REGS.len())
.map(|_| mir.alloc_vreg())
.collect();

define_loaded_values(&mut mir, &vregs);
mir.push(Aarch64Inst::Bl {
symbol_id: symbol,
returns: rue_runtime_abi::ReturnBehavior::Never,
});
for &vreg in &vregs {
mir.push(Aarch64Inst::MovRR {
dst: Operand::Physical(Reg::X0),
src: Operand::Virtual(vreg),
});
}

let (mir, num_spills, _) = RegAlloc::new(mir, 0).allocate_with_spills().unwrap();

assert_eq!(
num_spills, 0,
"the trap is not a barrier, so every value should still fit in a register"
);
let assigned: Vec<Reg> = mir
.instructions()
.iter()
.filter_map(|inst| match inst {
Aarch64Inst::Ldr {
dst: Operand::Physical(reg),
base: Reg::X1,
..
} => Some(*reg),
_ => None,
})
.collect();
for &reg in CALLER_SAVED_REGS {
assert!(
assigned.contains(&reg),
"a value spanning only the trap should be allowed caller-saved {reg}"
);
}
}

#[test]
fn a_never_returning_call_ends_liveness() {
// Modelling the trap as non-returning is what makes the clobber test
// above correct: nothing propagates past it.
let mut mir = Aarch64Mir::new();
let value = mir.alloc_vreg();
let symbol = mir.intern_symbol(rue_runtime_abi::RuntimeHelperId::Overflow.symbol());

mir.push(Aarch64Inst::MovImm {
dst: Operand::Virtual(value),
imm: 42,
});
mir.push(Aarch64Inst::Bl {
symbol_id: symbol,
returns: rue_runtime_abi::ReturnBehavior::Never,
});
mir.push(Aarch64Inst::MovRR {
dst: Operand::Physical(Reg::X0),
src: Operand::Virtual(value),
});
mir.push(Aarch64Inst::Ret);

let debug = liveness::analyze_debug(&mir);
assert!(
debug.instructions[1].live_out.is_empty(),
"a never-returning call must have an empty live-out set; found: {:?}",
debug.instructions[1].live_out
);
}

#[test]
#[should_panic(expected = "lowering named the allocatable register")]
fn lowering_may_not_name_an_allocatable_register_as_a_physical_operand() {
let mut mir = Aarch64Mir::new();
let value = mir.alloc_vreg();
mir.push(Aarch64Inst::MovRR {
dst: Operand::Virtual(value),
src: Operand::Physical(ALLOCATABLE_REGS[0]),
});

let _ = RegAlloc::new(mir, 0).allocate();
}

#[test]
fn test_call_survival_and_symbol_reconstruction() {
let mut mir = Aarch64Mir::new();
Expand All @@ -1986,7 +2087,7 @@ mod tests {
dst: Operand::Virtual(value),
imm: 42,
});
mir.push(Aarch64Inst::Bl { symbol_id: symbol });
mir.push(Aarch64Inst::call(symbol));
mir.push(Aarch64Inst::MovRR {
dst: Operand::Physical(Reg::X0),
src: Operand::Virtual(value),
Expand All @@ -2007,9 +2108,9 @@ mod tests {

assert!(value_reg.is_callee_saved());
assert!(
mir.instructions()
.iter()
.any(|inst| matches!(inst, Aarch64Inst::Bl { symbol_id } if *symbol_id == symbol))
mir.instructions().iter().any(
|inst| matches!(inst, Aarch64Inst::Bl { symbol_id, .. } if *symbol_id == symbol)
)
);
assert!(mir.instructions().iter().any(|inst| matches!(
inst,
Expand Down
6 changes: 3 additions & 3 deletions crates/rue-codegen/src/aarch64/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ fn accesses_memory(inst: &Aarch64Inst) -> bool {
}

/// Get registers read by an instruction (for dependency analysis).
fn regs_read(inst: &Aarch64Inst) -> Vec<Reg> {
pub(super) fn regs_read(inst: &Aarch64Inst) -> Vec<Reg> {
let mut result = Vec::new();

let add_if_phys = |op: &Operand, vec: &mut Vec<Reg>| {
Expand Down Expand Up @@ -364,7 +364,7 @@ fn regs_read(inst: &Aarch64Inst) -> Vec<Reg> {
}

/// Get registers written by an instruction (for dependency analysis).
fn regs_written(inst: &Aarch64Inst) -> Vec<Reg> {
pub(super) fn regs_written(inst: &Aarch64Inst) -> Vec<Reg> {
let mut result = Vec::new();

let add_if_phys = |op: &Operand, vec: &mut Vec<Reg>| {
Expand Down Expand Up @@ -620,7 +620,7 @@ mod tests {
id: LabelId::new(0)
}));
assert!(is_barrier(&Aarch64Inst::Ret));
assert!(is_barrier(&Aarch64Inst::Bl { symbol_id: 0 }));
assert!(is_barrier(&Aarch64Inst::call(0)));

assert!(!is_barrier(&Aarch64Inst::MovRR {
dst: Operand::Physical(Reg::X0),
Expand Down
Loading