Skip to content

Commit

Permalink
Cranelift: Remove the old stack maps implementation (#9159)
Browse files Browse the repository at this point in the history 
They are superseded by the new user stack maps implementation.
  • Loading branch information
@fitzgen
fitzgen authored Aug 21, 2024
1 parent 2389dcc commit c0c3a68
Show file tree
Hide file tree
Showing 17 changed files with 99 additions and 417 deletions.
3 changes: 0 additions & 3 deletions cranelift/codegen/src/binemit/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@
//! The `binemit` module contains code for translating Cranelift's intermediate representation into
//! binary machine code.

mod stack_map;

pub use self::stack_map::StackMap;
use core::fmt;
#[cfg(feature = "enable-serde")]
use serde_derive::{Deserialize, Serialize};
Expand Down
141 changes: 0 additions & 141 deletions cranelift/codegen/src/binemit/stack_map.rs
View file

This file was deleted.

62 changes: 62 additions & 0 deletions cranelift/codegen/src/ir/user_stack_maps.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,68 @@
//! `cranelift_codegen::ir::DataFlowGraph::append_user_stack_map_entry`). Cranelift
//! will not insert spills and record these stack map entries automatically (in
//! contrast to the old system and its `r64` values).
//!
//! Logically, a set of stack maps for a function record a table of the form:
//!
//! ```text
//! +---------------------+-------------------------------------------+
//! | Instruction Pointer | SP-Relative Offsets of Live GC References |
//! +---------------------+-------------------------------------------+
//! | 0x12345678 | 2, 6, 12 |
//! | 0x1234abcd | 2, 6 |
//! | ... | ... |
//! +---------------------+-------------------------------------------+
//! ```
//!
//! Where "instruction pointer" is an instruction pointer within the function,
//! and "offsets of live GC references" contains the offsets (in units of words)
//! from the frame's stack pointer where live GC references are stored on the
//! stack. Instruction pointers within the function that do not have an entry in
//! this table are not GC safepoints.
//!
//! Because
//!
//! * offsets of live GC references are relative from the stack pointer, and
//! * stack frames grow down from higher addresses to lower addresses,
//!
//! to get a pointer to a live reference at offset `x` within a stack frame, you
//! add `x` to the frame's stack pointer.
//!
//! For example, to calculate the pointer to the live GC reference inside "frame
//! 1" below, you would do `frame_1_sp + x`:
//!
//! ```text
//! Stack
//! +-------------------+
//! | Frame 0 |
//! | |
//! | | |
//! | +-------------------+ <--- Frame 0's SP
//! | | Frame 1 |
//! Grows | |
//! down | |
//! | | Live GC reference | --+--
//! | | | |
//! | | | |
//! V | | x = offset of live GC reference
//! | | |
//! | | |
//! +-------------------+ --+-- <--- Frame 1's SP
//! | Frame 2 |
//! | ... |
//! ```
//!
//! An individual `UserStackMap` is associated with just one instruction pointer
//! within the function, contains the size of the stack frame, and represents
//! the stack frame as a bitmap. There is one bit per word in the stack frame,
//! and if the bit is set, then the word contains a live GC reference.
//!
//! Note that a caller's outgoing argument stack slots (if any) and callee's
//! incoming argument stack slots (if any) overlap, so we must choose which
//! function's stack maps record live GC references in these slots. We record
//! the incoming arguments in the callee's stack map. This choice plays nice
//! with tail calls, where by the time we transfer control to the callee, the
//! caller no longer exists.

use crate::ir;
use cranelift_bitset::CompoundBitSet;
Expand Down
29 changes: 6 additions & 23 deletions cranelift/codegen/src/isa/aarch64/inst/emit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

use cranelift_control::ControlPlane;

use crate::binemit::StackMap;
use crate::ir::{self, types::*};
use crate::isa::aarch64::inst::*;
use crate::trace;
Expand Down Expand Up @@ -651,10 +650,6 @@ fn enc_asimd_mod_imm(rd: Writable<Reg>, q_op: u32, cmode: u32, imm: u8) -> u32 {
/// State carried between emissions of a sequence of instructions.
#[derive(Default, Clone, Debug)]
pub struct EmitState {
/// Safepoint stack map for upcoming instruction, as provided to
/// `pre_safepoint()`.
stack_map: Option<StackMap>,

/// The user stack map for the upcoming instruction, as provided to
/// `pre_safepoint()`.
user_stack_map: Option<ir::UserStackMap>,
Expand All @@ -669,19 +664,13 @@ pub struct EmitState {
impl MachInstEmitState<Inst> for EmitState {
fn new(abi: &Callee<AArch64MachineDeps>, ctrl_plane: ControlPlane) -> Self {
EmitState {
stack_map: None,
user_stack_map: None,
ctrl_plane,
frame_layout: abi.frame_layout().clone(),
}
}

fn pre_safepoint(
&mut self,
stack_map: Option<StackMap>,
user_stack_map: Option<ir::UserStackMap>,
) {
self.stack_map = stack_map;
fn pre_safepoint(&mut self, user_stack_map: Option<ir::UserStackMap>) {
self.user_stack_map = user_stack_map;
}

Expand All @@ -699,12 +688,12 @@ impl MachInstEmitState<Inst> for EmitState {
}

impl EmitState {
fn take_stack_map(&mut self) -> (Option<StackMap>, Option<ir::UserStackMap>) {
(self.stack_map.take(), self.user_stack_map.take())
fn take_stack_map(&mut self) -> Option<ir::UserStackMap> {
self.user_stack_map.take()
}

fn clear_post_insn(&mut self) {
self.stack_map = None;
self.user_stack_map = None;
}
}

Expand Down Expand Up @@ -2935,10 +2924,7 @@ impl MachInstEmit for Inst {
}
}
&Inst::Call { ref info } => {
let (stack_map, user_stack_map) = state.take_stack_map();
if let Some(s) = stack_map {
sink.add_stack_map(StackMapExtent::UpcomingBytes(4), s);
}
let user_stack_map = state.take_stack_map();
sink.add_reloc(Reloc::Arm64Call, &info.dest, 0);
sink.put4(enc_jump26(0b100101, 0));
if let Some(s) = user_stack_map {
Expand All @@ -2956,10 +2942,7 @@ impl MachInstEmit for Inst {
}
}
&Inst::CallInd { ref info } => {
let (stack_map, user_stack_map) = state.take_stack_map();
if let Some(s) = stack_map {
sink.add_stack_map(StackMapExtent::UpcomingBytes(4), s);
}
let user_stack_map = state.take_stack_map();
let rn = info.rn;
sink.put4(0b1101011_0001_11111_000000_00000_00000 | (machreg_to_gpr(rn) << 5));
if let Some(s) = user_stack_map {
Expand Down
20 changes: 4 additions & 16 deletions cranelift/codegen/src/isa/pulley_shared/inst/emit.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Pulley binary code emission.

use super::*;
use crate::binemit::StackMap;
use crate::ir;
use crate::isa::pulley_shared::abi::PulleyMachineDeps;
use crate::isa::pulley_shared::PointerWidth;
Expand Down Expand Up @@ -40,7 +39,6 @@ where
{
_phantom: PhantomData<P>,
ctrl_plane: ControlPlane,
stack_map: Option<StackMap>,
user_stack_map: Option<ir::UserStackMap>,
pub virtual_sp_offset: i64,
frame_layout: FrameLayout,
Expand All @@ -50,8 +48,8 @@ impl<P> EmitState<P>
where
P: PulleyTargetKind,
{
fn take_stack_map(&mut self) -> (Option<StackMap>, Option<ir::UserStackMap>) {
(self.stack_map.take(), self.user_stack_map.take())
fn take_stack_map(&mut self) -> Option<ir::UserStackMap> {
self.user_stack_map.take()
}

pub(crate) fn adjust_virtual_sp_offset(&mut self, amount: i64) {
Expand All @@ -70,19 +68,13 @@ where
EmitState {
_phantom: PhantomData,
ctrl_plane,
stack_map: None,
user_stack_map: None,
virtual_sp_offset: 0,
frame_layout: abi.frame_layout().clone(),
}
}

fn pre_safepoint(
&mut self,
stack_map: Option<StackMap>,
user_stack_map: Option<ir::UserStackMap>,
) {
self.stack_map = stack_map;
fn pre_safepoint(&mut self, user_stack_map: Option<ir::UserStackMap>) {
self.user_stack_map = user_stack_map;
}

Expand Down Expand Up @@ -156,10 +148,6 @@ fn pulley_emit<P>(
Inst::LoadExtName { .. } => todo!(),

Inst::Call { callee, info } => {
let (stack_map, user_stack_map) = state.take_stack_map();
if let Some(s) = stack_map {
sink.add_stack_map(StackMapExtent::UpcomingBytes(5), s);
}
sink.put1(pulley_interpreter::Opcode::Call as u8);
sink.add_reloc(
// TODO: is it actually okay to reuse this reloc here?
Expand All @@ -170,7 +158,7 @@ fn pulley_emit<P>(
-1,
);
sink.put4(0);
if let Some(s) = user_stack_map {
if let Some(s) = state.take_stack_map() {
let offset = sink.cur_offset();
sink.push_user_stack_map(state, offset, s);
}
Expand Down
Loading

0 comments on commit c0c3a68

Please sign in to comment.