Skip to content

Commit

Permalink
Cranelift: Don't attempt to take stack maps for traps during instruct…
Browse files Browse the repository at this point in the history
…ion emission (#8812)

Forgot to address this in #8810
  • Loading branch information
fitzgen authored Jun 14, 2024
1 parent 9aa5803 commit b7fd095
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 40 deletions.
5 changes: 1 addition & 4 deletions cranelift/codegen/src/isa/aarch64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3037,7 +3037,7 @@ 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());
let label = sink.defer_trap(trap_code);
// condbr KIND, LABEL
let off = sink.cur_offset();
sink.put4(enc_conditional_br(BranchTarget::Label(label), kind));
Expand All @@ -3055,9 +3055,6 @@ impl MachInstEmit for Inst {
}
&Inst::Udf { trap_code } => {
sink.add_trap(trap_code);
if let Some(s) = state.take_stack_map() {
sink.add_stack_map(StackMapExtent::UpcomingBytes(4), s);
}
sink.put_data(Inst::TRAP_OPCODE);
}
&Inst::Adr { rd, off } => {
Expand Down
9 changes: 0 additions & 9 deletions cranelift/codegen/src/isa/riscv64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,9 +398,6 @@ impl Inst {
// c.unimp
Inst::Udf { trap_code } => {
sink.add_trap(trap_code);
if let Some(s) = state.take_stack_map() {
sink.add_stack_map(StackMapExtent::UpcomingBytes(2), s);
}
sink.put2(0x0000);
}

Expand Down Expand Up @@ -2038,12 +2035,6 @@ impl Inst {
}
&Inst::Udf { trap_code } => {
sink.add_trap(trap_code);
if let Some(s) = state.take_stack_map() {
sink.add_stack_map(
StackMapExtent::UpcomingBytes(Inst::TRAP_OPCODE.len() as u32),
s,
);
}
sink.put_data(Inst::TRAP_OPCODE);
}
&Inst::AtomicLoad { rd, ty, p } => {
Expand Down
6 changes: 0 additions & 6 deletions cranelift/codegen/src/isa/s390x/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3317,15 +3317,9 @@ impl Inst {
put(sink, &enc_e(0x0001));
}
&Inst::Trap { trap_code } => {
if let Some(s) = state.take_stack_map() {
sink.add_stack_map(StackMapExtent::UpcomingBytes(2), s);
}
put_with_trap(sink, &enc_e(0x0000), trap_code);
}
&Inst::TrapIf { cond, trap_code } => {
if let Some(s) = state.take_stack_map() {
sink.add_stack_map(StackMapExtent::UpcomingBytes(6), s);
}
// We implement a TrapIf as a conditional branch into the middle
// of the branch (BRCL) instruction itself - those middle two bytes
// are zero, which matches the trap instruction itself.
Expand Down
9 changes: 3 additions & 6 deletions cranelift/codegen/src/isa/x64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1895,7 +1895,7 @@ pub(crate) fn emit(
}

Inst::TrapIf { cc, trap_code } => {
let trap_label = sink.defer_trap(*trap_code, state.take_stack_map());
let trap_label = sink.defer_trap(*trap_code);
one_way_jmp(sink, *cc, trap_label);
}

Expand All @@ -1904,7 +1904,7 @@ pub(crate) fn emit(
cc2,
trap_code,
} => {
let trap_label = sink.defer_trap(*trap_code, state.take_stack_map());
let trap_label = sink.defer_trap(*trap_code);
let else_label = sink.get_label();

// Jump to the end if the first condition isn't true, and then if
Expand All @@ -1920,7 +1920,7 @@ pub(crate) fn emit(
cc2,
trap_code,
} => {
let trap_label = sink.defer_trap(*trap_code, state.take_stack_map());
let trap_label = sink.defer_trap(*trap_code);

// Emit two jumps to the same trap if either condition code is true.
one_way_jmp(sink, *cc1, trap_label);
Expand Down Expand Up @@ -4021,9 +4021,6 @@ pub(crate) fn emit(

Inst::Ud2 { trap_code } => {
sink.add_trap(*trap_code);
if let Some(s) = state.take_stack_map() {
sink.add_stack_map(StackMapExtent::UpcomingBytes(2), s);
}
sink.put_data(Inst::TRAP_OPCODE);
}

Expand Down
17 changes: 2 additions & 15 deletions cranelift/codegen/src/machinst/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1219,12 +1219,11 @@ impl<I: VCodeInst> MachBuffer<I> {
/// patched in once the address of the trap is known.
///
/// This will batch all traps into the end of the function.
pub fn defer_trap(&mut self, code: TrapCode, stack_map: Option<StackMap>) -> MachLabel {
pub fn defer_trap(&mut self, code: TrapCode) -> MachLabel {
let label = self.get_label();
self.pending_traps.push(MachLabelTrap {
label,
code,
stack_map,
loc: self.cur_srcloc.map(|(_start, loc)| loc),
});
label
Expand Down Expand Up @@ -1299,13 +1298,7 @@ impl<I: VCodeInst> MachBuffer<I> {
// Note that traps are placed first since this typically happens at the
// end of the function and for disassemblers we try to keep all the code
// contiguously together.
for MachLabelTrap {
label,
code,
stack_map,
loc,
} in mem::take(&mut self.pending_traps)
{
for MachLabelTrap { label, code, loc } in mem::take(&mut self.pending_traps) {
// If this trap has source information associated with it then
// emit this information for the trap instruction going out now too.
if let Some(loc) = loc {
Expand All @@ -1314,10 +1307,6 @@ impl<I: VCodeInst> MachBuffer<I> {
self.align_to(I::LabelUse::ALIGN);
self.bind_label(label, ctrl_plane);
self.add_trap(code);
if let Some(map) = stack_map {
let extent = StackMapExtent::UpcomingBytes(I::TRAP_OPCODE.len() as u32);
self.add_stack_map(extent, map);
}
self.put_data(I::TRAP_OPCODE);
if loc.is_some() {
self.end_srcloc();
Expand Down Expand Up @@ -1760,8 +1749,6 @@ struct MachLabelTrap {
label: MachLabel,
/// The code associated with this trap.
code: TrapCode,
/// An optional stack map to associate with this trap.
stack_map: Option<StackMap>,
/// An optional source location to assign for this trap.
loc: Option<RelSourceLoc>,
}
Expand Down

0 comments on commit b7fd095

Please sign in to comment.