Skip to content

Commit

Permalink
Add a MachBuffer::defer_trap method (#6011)
Browse files Browse the repository at this point in the history 
* Add a `MachBuffer::defer_trap` method

This commit adds a new method to `MachBuffer` to defer trap opcodes to
the end of a function in a similar manner to how constants are deferred
to the end of the function. This is useful for backends which frequently
use `TrapIf`-style opcodes. Currently a jump is emitted which skips the
next instruction, a trap, and then execution continues normally. While
there isn't any pressing problem with this construction the trap opcode
is in the middle of the instruction stream as opposed to "off on the
side" despite rarely being taken.

With this method in place all the backends (except riscv64 since I
couldn't figure it out easily enough) have a new lowering of their
`TrapIf` opcode. Now a trap is deferred, which returns a label, and then
that label is jumped to when executing the trap. A fixup is then
recorded in `MachBuffer` to get patched later on during emission, or at
the end of the function. Subsequently all `TrapIf` instructions
translate to a single branch plus a single trap at the end of the
function.

I've additionally further updated some more lowerings in the x64 backend
which were explicitly using traps to instead use `TrapIf` where
applicable to avoid jumping over traps mid-function. Other backends
didn't appear to have many jump-over-the-next-trap patterns.

Lots of tests have had their expectations updated here which should
reflect all the traps being sunk to the end of functions.

* Print trap code on all platforms

* Emit traps before constants

* Preserve source location information for traps

* Fix test expectations

* Attempt to fix s390x

The MachBuffer was registering trap codes with the first byte of the
trap, but the SIGILL handler was expecting it to be registered with the
last byte of the trap. Exploit that SIGILL is always represented with a
2-byte instruction and always march 2-backwards for SIGILL, continuing
to march backwards 1 byte for SIGFPE-generating instructions.

* Back out s390x changes

* Back out more s390x bits

* Review comments
  • Loading branch information
@alexcrichton
alexcrichton authored Mar 20, 2023
1 parent 6a03398 commit a3b2103
Show file tree
Hide file tree
Showing 52 changed files with 702 additions and 588 deletions.
15 changes: 3 additions & 12 deletions cranelift/codegen/src/isa/aarch64/inst/emit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3115,20 +3115,15 @@ impl MachInstEmit for Inst {
sink.put4(enc_jump26(0b000101, not_taken.as_offset26_or_zero()));
}
&Inst::TrapIf { kind, trap_code } => {
let label = sink.defer_trap(trap_code, state.take_stack_map());
// condbr KIND, LABEL
let off = sink.cur_offset();
let label = sink.get_label();
sink.put4(enc_conditional_br(
BranchTarget::Label(label),
kind.invert(),
kind,
&mut allocs,
));
sink.use_label_at_offset(off, label, LabelUse::Branch19);
// udf
let trap = Inst::Udf { trap_code };
trap.emit(&[], sink, emit_info, state);
// LABEL:
sink.bind_label(label);
}
&Inst::IndirectBr { rn, .. } => {
let rn = allocs.next(rn);
Expand All @@ -3142,15 +3137,11 @@ impl MachInstEmit for Inst {
sink.put4(0xd4200000);
}
&Inst::Udf { trap_code } => {
// "CLIF" in hex, to make the trap recognizable during
// debugging.
let encoding = 0xc11f;

sink.add_trap(trap_code);
if let Some(s) = state.take_stack_map() {
sink.add_stack_map(StackMapExtent::UpcomingBytes(4), s);
}
sink.put4(encoding);
sink.put_data(Inst::TRAP_OPCODE);
}
&Inst::Adr { rd, off } => {
let rd = allocs.next_writable(rd);
Expand Down
72 changes: 36 additions & 36 deletions cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5873,144 +5873,144 @@ fn test_aarch64_binemit() {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::NotZero(xreg(8)),
},
"480000B41FC10000",
"cbz x8, 8 ; udf",
"280000B51FC10000",
"cbnz x8, #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Zero(xreg(8)),
},
"480000B51FC10000",
"cbnz x8, 8 ; udf",
"280000B41FC10000",
"cbz x8, #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Ne),
},
"400000541FC10000",
"b.eq 8 ; udf",
"210000541FC10000",
"b.ne #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Eq),
},
"410000541FC10000",
"b.ne 8 ; udf",
"200000541FC10000",
"b.eq #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Lo),
},
"420000541FC10000",
"b.hs 8 ; udf",
"230000541FC10000",
"b.lo #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Hs),
},
"430000541FC10000",
"b.lo 8 ; udf",
"220000541FC10000",
"b.hs #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Pl),
},
"440000541FC10000",
"b.mi 8 ; udf",
"250000541FC10000",
"b.pl #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Mi),
},
"450000541FC10000",
"b.pl 8 ; udf",
"240000541FC10000",
"b.mi #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Vc),
},
"460000541FC10000",
"b.vs 8 ; udf",
"270000541FC10000",
"b.vc #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Vs),
},
"470000541FC10000",
"b.vc 8 ; udf",
"260000541FC10000",
"b.vs #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Ls),
},
"480000541FC10000",
"b.hi 8 ; udf",
"290000541FC10000",
"b.ls #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Hi),
},
"490000541FC10000",
"b.ls 8 ; udf",
"280000541FC10000",
"b.hi #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Lt),
},
"4A0000541FC10000",
"b.ge 8 ; udf",
"2B0000541FC10000",
"b.lt #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Ge),
},
"4B0000541FC10000",
"b.lt 8 ; udf",
"2A0000541FC10000",
"b.ge #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Le),
},
"4C0000541FC10000",
"b.gt 8 ; udf",
"2D0000541FC10000",
"b.le #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Gt),
},
"4D0000541FC10000",
"b.le 8 ; udf",
"2C0000541FC10000",
"b.gt #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Nv),
},
"4E0000541FC10000",
"b.al 8 ; udf",
"2F0000541FC10000",
"b.nv #trap=interrupt",
));
insns.push((
Inst::TrapIf {
trap_code: TrapCode::Interrupt,
kind: CondBrKind::Cond(Cond::Al),
},
"4F0000541FC10000",
"b.nv 8 ; udf",
"2E0000541FC10000",
"b.al #trap=interrupt",
));

insns.push((
Expand Down
17 changes: 12 additions & 5 deletions cranelift/codegen/src/isa/aarch64/inst/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -926,6 +926,10 @@ impl MachInst for Inst {
type ABIMachineSpec = AArch64MachineDeps;
type LabelUse = LabelUse;

// "CLIF" in hex, to make the trap recognizable during
// debugging.
const TRAP_OPCODE: &'static [u8] = &0xc11f_u32.to_le_bytes();

fn get_operands<F: Fn(VReg) -> VReg>(&self, collector: &mut OperandCollector<'_, F>) {
aarch64_get_operands(self, collector);
}
Expand Down Expand Up @@ -2519,18 +2523,21 @@ impl Inst {
}
&Inst::Brk => "brk #0".to_string(),
&Inst::Udf { .. } => "udf #0xc11f".to_string(),
&Inst::TrapIf { ref kind, .. } => match kind {
&Inst::TrapIf {
ref kind,
trap_code,
} => match kind {
&CondBrKind::Zero(reg) => {
let reg = pretty_print_reg(reg, allocs);
format!("cbnz {}, 8 ; udf", reg)
format!("cbz {reg}, #trap={trap_code}")
}
&CondBrKind::NotZero(reg) => {
let reg = pretty_print_reg(reg, allocs);
format!("cbz {}, 8 ; udf", reg)
format!("cbnz {reg}, #trap={trap_code}")
}
&CondBrKind::Cond(c) => {
let c = c.invert().pretty_print(0, allocs);
format!("b.{} 8 ; udf", c)
let c = c.pretty_print(0, allocs);
format!("b.{c} #trap={trap_code}")
}
},
&Inst::Adr { rd, off } => {
Expand Down
4 changes: 1 addition & 3 deletions cranelift/codegen/src/isa/riscv64/inst/emit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1908,9 +1908,7 @@ impl MachInstEmit for Inst {
if let Some(s) = state.take_stack_map() {
sink.add_stack_map(StackMapExtent::UpcomingBytes(4), s);
}
// https://github.com/riscv/riscv-isa-manual/issues/850
// all zero will cause invalid opcode.
sink.put4(0);
sink.put_data(Inst::TRAP_OPCODE);
}
&Inst::SelectIf {
if_spectre_guard: _if_spectre_guard, // _if_spectre_guard not use because it is used to not be removed by optimization pass and some other staff.
Expand Down
4 changes: 4 additions & 0 deletions cranelift/codegen/src/isa/riscv64/inst/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,10 @@ impl MachInst for Inst {
type LabelUse = LabelUse;
type ABIMachineSpec = Riscv64MachineDeps;

// https://github.com/riscv/riscv-isa-manual/issues/850
// all zero will cause invalid opcode.
const TRAP_OPCODE: &'static [u8] = &[0; 4];

fn gen_dummy_use(reg: Reg) -> Self {
Inst::DummyUse { reg }
}
Expand Down
1 change: 1 addition & 0 deletions cranelift/codegen/src/isa/s390x/inst/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,7 @@ fn s390x_get_operands<F: Fn(VReg) -> VReg>(inst: &Inst, collector: &mut OperandC
impl MachInst for Inst {
type ABIMachineSpec = S390xMachineDeps;
type LabelUse = LabelUse;
const TRAP_OPCODE: &'static [u8] = &[0, 0];

fn get_operands<F: Fn(VReg) -> VReg>(&self, collector: &mut OperandCollector<'_, F>) {
s390x_get_operands(self, collector);
Expand Down
Loading

0 comments on commit a3b2103

Please sign in to comment.