AArch64: avoid branches with explicit offsets at lowering stage.
In discussions with @bnjbvr, it came up that generating `OneWayCondBr`s with explicit, hardcoded PC-offsets as part of lowered instruction sequences is actually unsafe, because the register allocator *might* insert a spill or reload into the middle of our sequence. We were careful about this in some cases but somehow missed that it was a general restriction. Conceptually, all inter-instruction references should be via labels at the VCode level; explicit offsets are only ever known at emission time, and resolved by the `MachBuffer`. To allow for conditional trap checks without modifying the CFG (as seen by regalloc) during lowering, this PR instead adds a `TrapIf` pseudo-instruction that conditionally skips a single embedded trap instruction. It lowers to the same `condbr label ; trap ; label: ...` sequence, but without the hardcoded branch-target offset in the lowering code.
This commit is contained in:
Chris Fallin
@@ -334,6 +334,7 @@ pub struct CallIndInfo {
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct JTSequenceInfo {
|
||||
pub targets: Vec<BranchTarget>,
|
||||
pub default_target: BranchTarget,
|
||||
pub targets_for_term: Vec<MachLabel>, // needed for MachTerminator.
|
||||
}
|
||||
|
||||
@@ -817,20 +818,17 @@ pub enum Inst {
|
||||
kind: CondBrKind,
|
||||
},
|
||||
|
||||
/// A one-way conditional branch, invisible to the CFG processing; used *only* as part of
|
||||
/// straight-line sequences in code to be emitted.
|
||||
/// A conditional trap: execute a `udf` if the condition is true. This is
|
||||
/// one VCode instruction because it uses embedded control flow; it is
|
||||
/// logically a single-in, single-out region, but needs to appear as one
|
||||
/// unit to the register allocator.
|
||||
///
|
||||
/// In more detail:
|
||||
/// - This branch is lowered to a branch at the machine-code level, but does not end a basic
|
||||
/// block, and does not create edges in the CFG seen by regalloc.
|
||||
/// - Thus, it is *only* valid to use as part of a single-in, single-out sequence that is
|
||||
/// lowered from a single CLIF instruction. For example, certain arithmetic operations may
|
||||
/// use these branches to handle certain conditions, such as overflows, traps, etc.
|
||||
///
|
||||
/// See, e.g., the lowering of `trapif` (conditional trap) for an example.
|
||||
OneWayCondBr {
|
||||
target: BranchTarget,
|
||||
/// The `CondBrKind` gives the conditional-branch condition that will
|
||||
/// *execute* the embedded `Inst`. (In the emitted code, we use the inverse
|
||||
/// of this condition in a branch that skips the trap instruction.)
|
||||
TrapIf {
|
||||
kind: CondBrKind,
|
||||
trap_info: (SourceLoc, TrapCode),
|
||||
},
|
||||
|
||||
/// An indirect branch through a register, augmented with set of all
|
||||
@@ -1346,7 +1344,7 @@ fn aarch64_get_regs(inst: &Inst, collector: &mut RegUsageCollector) {
|
||||
collector.add_defs(&*info.defs);
|
||||
collector.add_use(info.rn);
|
||||
}
|
||||
&Inst::CondBr { ref kind, .. } | &Inst::OneWayCondBr { ref kind, .. } => match kind {
|
||||
&Inst::CondBr { ref kind, .. } => match kind {
|
||||
CondBrKind::Zero(rt) | CondBrKind::NotZero(rt) => {
|
||||
collector.add_use(*rt);
|
||||
}
|
||||
@@ -1358,6 +1356,12 @@ fn aarch64_get_regs(inst: &Inst, collector: &mut RegUsageCollector) {
|
||||
&Inst::Nop0 | Inst::Nop4 => {}
|
||||
&Inst::Brk => {}
|
||||
&Inst::Udf { .. } => {}
|
||||
&Inst::TrapIf { ref kind, .. } => match kind {
|
||||
CondBrKind::Zero(rt) | CondBrKind::NotZero(rt) => {
|
||||
collector.add_use(*rt);
|
||||
}
|
||||
CondBrKind::Cond(_) => {}
|
||||
},
|
||||
&Inst::Adr { rd, .. } => {
|
||||
collector.add_def(rd);
|
||||
}
|
||||
@@ -1949,13 +1953,16 @@ fn aarch64_map_regs<RUM: RegUsageMapper>(inst: &mut Inst, mapper: &RUM) {
|
||||
}
|
||||
map_use(mapper, &mut info.rn);
|
||||
}
|
||||
&mut Inst::CondBr { ref mut kind, .. } | &mut Inst::OneWayCondBr { ref mut kind, .. } => {
|
||||
&mut Inst::CondBr { ref mut kind, .. } => {
|
||||
map_br(mapper, kind);
|
||||
}
|
||||
&mut Inst::IndirectBr { ref mut rn, .. } => {
|
||||
map_use(mapper, rn);
|
||||
}
|
||||
&mut Inst::Nop0 | &mut Inst::Nop4 | &mut Inst::Brk | &mut Inst::Udf { .. } => {}
|
||||
&mut Inst::TrapIf { ref mut kind, .. } => {
|
||||
map_br(mapper, kind);
|
||||
}
|
||||
&mut Inst::Adr { ref mut rd, .. } => {
|
||||
map_def(mapper, rd);
|
||||
}
|
||||
@@ -2026,10 +2033,6 @@ impl MachInst for Inst {
|
||||
&Inst::CondBr {
|
||||
taken, not_taken, ..
|
||||
} => MachTerminator::Cond(taken.as_label().unwrap(), not_taken.as_label().unwrap()),
|
||||
&Inst::OneWayCondBr { .. } => {
|
||||
// Explicitly invisible to CFG processing.
|
||||
MachTerminator::None
|
||||
}
|
||||
&Inst::IndirectBr { ref targets, .. } => MachTerminator::Indirect(&targets[..]),
|
||||
&Inst::JTSequence { ref info, .. } => {
|
||||
MachTerminator::Indirect(&info.targets_for_term[..])
|
||||
@@ -2880,32 +2883,26 @@ impl ShowWithRRU for Inst {
|
||||
}
|
||||
}
|
||||
}
|
||||
&Inst::OneWayCondBr {
|
||||
ref target,
|
||||
ref kind,
|
||||
} => {
|
||||
let target = target.show_rru(mb_rru);
|
||||
match kind {
|
||||
&CondBrKind::Zero(reg) => {
|
||||
let reg = reg.show_rru(mb_rru);
|
||||
format!("cbz {}, {}", reg, target)
|
||||
}
|
||||
&CondBrKind::NotZero(reg) => {
|
||||
let reg = reg.show_rru(mb_rru);
|
||||
format!("cbnz {}, {}", reg, target)
|
||||
}
|
||||
&CondBrKind::Cond(c) => {
|
||||
let c = c.show_rru(mb_rru);
|
||||
format!("b.{} {}", c, target)
|
||||
}
|
||||
}
|
||||
}
|
||||
&Inst::IndirectBr { rn, .. } => {
|
||||
let rn = rn.show_rru(mb_rru);
|
||||
format!("br {}", rn)
|
||||
}
|
||||
&Inst::Brk => "brk #0".to_string(),
|
||||
&Inst::Udf { .. } => "udf".to_string(),
|
||||
&Inst::TrapIf { ref kind, .. } => match kind {
|
||||
&CondBrKind::Zero(reg) => {
|
||||
let reg = reg.show_rru(mb_rru);
|
||||
format!("cbnz {}, 8 ; udf", reg)
|
||||
}
|
||||
&CondBrKind::NotZero(reg) => {
|
||||
let reg = reg.show_rru(mb_rru);
|
||||
format!("cbz {}, 8 ; udf", reg)
|
||||
}
|
||||
&CondBrKind::Cond(c) => {
|
||||
let c = c.invert().show_rru(mb_rru);
|
||||
format!("b.{} 8 ; udf", c)
|
||||
}
|
||||
},
|
||||
&Inst::Adr { rd, off } => {
|
||||
let rd = rd.show_rru(mb_rru);
|
||||
format!("adr {}, pc+{}", rd, off)
|
||||
@@ -2922,15 +2919,26 @@ impl ShowWithRRU for Inst {
|
||||
let ridx = ridx.show_rru(mb_rru);
|
||||
let rtmp1 = rtmp1.show_rru(mb_rru);
|
||||
let rtmp2 = rtmp2.show_rru(mb_rru);
|
||||
let default_target = info.default_target.show_rru(mb_rru);
|
||||
format!(
|
||||
concat!(
|
||||
"b.hs {} ; ",
|
||||
"adr {}, pc+16 ; ",
|
||||
"ldrsw {}, [{}, {}, LSL 2] ; ",
|
||||
"add {}, {}, {} ; ",
|
||||
"br {} ; ",
|
||||
"jt_entries {:?}"
|
||||
),
|
||||
rtmp1, rtmp2, rtmp1, ridx, rtmp1, rtmp1, rtmp2, rtmp1, info.targets
|
||||
default_target,
|
||||
rtmp1,
|
||||
rtmp2,
|
||||
rtmp1,
|
||||
ridx,
|
||||
rtmp1,
|
||||
rtmp1,
|
||||
rtmp2,
|
||||
rtmp1,
|
||||
info.targets
|
||||
)
|
||||
}
|
||||
&Inst::LoadConst64 { rd, const_data } => {
|
||||
|
||||
Reference in New Issue
Block a user