Skip to content

Commit 3e14d77

Browse files
cfallincfallin
committed
Review feedback: switch from inlined stackslot descriptor blobs to u64 keys.
1 parent 1c12a68 commit 3e14d77

File tree

17 files changed

+85
-104
lines changed

17 files changed

+85
-104
lines changed

‎cranelift/codegen/src/ir/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ pub use crate::ir::progpoint::ProgramPoint;
6666
pub use crate::ir::sourceloc::RelSourceLoc;
6767
pub use crate::ir::sourceloc::SourceLoc;
6868
pub use crate::ir::stackslot::{
69-
DynamicStackSlotData, DynamicStackSlots, StackSlotData, StackSlotKind, StackSlots,
69+
DynamicStackSlotData, DynamicStackSlots, StackSlotData, StackSlotKey, StackSlotKind, StackSlots,
7070
};
7171
pub use crate::ir::trapcode::TrapCode;
7272
pub use crate::ir::types::Type;

‎cranelift/codegen/src/ir/stackslot.rs‎

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
use crate::entity::PrimaryMap;
77
use crate::ir::StackSlot;
88
use crate::ir::entities::{DynamicStackSlot, DynamicType};
9-
use alloc::vec::Vec;
109
use core::fmt;
1110
use core::str::FromStr;
1211

@@ -71,25 +70,62 @@ pub struct StackSlotData {
7170
/// stack slot size or machine word size, as well.
7271
pub align_shift: u8,
7372

74-
/// Opaque stackslot metadata, passed through to compilation
75-
/// result metadata describing stackslot location.
73+
/// Opaque stackslot metadata handle, passed through to
74+
/// compilation result metadata describing stackslot location.
7675
///
7776
/// In the face of compiler transforms like inlining that may move
7877
/// stackslots between functions, when an embedder wants to
7978
/// externally observe stackslots, it needs a first-class way for
8079
/// the identity of stackslots to be carried along with the IR
81-
/// entities. This opaque data allows the embedder to do that.
82-
pub descriptor: Vec<u8>,
80+
/// entities. This opaque `StackSlotKey` allows the embedder to do
81+
/// so.
82+
pub key: Option<StackSlotKey>,
83+
}
84+
85+
/// An opaque key uniquely identifying a stack slot.
86+
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
87+
#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))]
88+
pub struct StackSlotKey(u64);
89+
impl StackSlotKey {
90+
/// Construct a [`StackSlotKey`] from raw bits.
91+
///
92+
/// An embedder can use any 64-bit value to describe a stack slot;
93+
/// there are no restrictions, and the value does not mean
94+
/// anything to Cranelift itself.
95+
pub fn new(value: u64) -> StackSlotKey {
96+
StackSlotKey(value)
97+
}
98+
99+
/// Get the raw bits from the [`StackSlotKey`].
100+
pub fn bits(&self) -> u64 {
101+
self.0
102+
}
83103
}
84104

85105
impl StackSlotData {
86106
/// Create a stack slot with the specified byte size and alignment.
87-
pub fn new(kind: StackSlotKind, size: StackSize, align_shift: u8, descriptor: Vec<u8>) -> Self {
107+
pub fn new(kind: StackSlotKind, size: StackSize, align_shift: u8) -> Self {
108+
Self {
109+
kind,
110+
size,
111+
align_shift,
112+
key: None,
113+
}
114+
}
115+
116+
/// Create a stack slot with the specified byte size and alignment
117+
/// and the given user-defined key.
118+
pub fn new_with_key(
119+
kind: StackSlotKind,
120+
size: StackSize,
121+
align_shift: u8,
122+
key: StackSlotKey,
123+
) -> Self {
88124
Self {
89125
kind,
90126
size,
91127
align_shift,
92-
descriptor,
128+
key: Some(key),
93129
}
94130
}
95131
}
@@ -101,12 +137,12 @@ impl fmt::Display for StackSlotData {
101137
} else {
102138
"".into()
103139
};
104-
let descriptor = if !self.descriptor.is_empty() {
105-
format!(", descriptor = {:?}", self.descriptor)
106-
} else {
107-
"".into()
140+
let key = match self.key {
141+
Some(value) => format!(", key = {}", value.bits()),
142+
None => "".into(),
108143
};
109-
write!(f, "{} {}{align_shift}{descriptor}", self.kind, self.size)
144+
145+
write!(f, "{} {}{align_shift}{key}", self.kind, self.size)
110146
}
111147
}
112148

@@ -159,18 +195,10 @@ mod tests {
159195
fn stack_slot() {
160196
let mut func = Function::new();
161197

162-
let ss0 = func.create_sized_stack_slot(StackSlotData::new(
163-
StackSlotKind::ExplicitSlot,
164-
4,
165-
0,
166-
vec![],
167-
));
168-
let ss1 = func.create_sized_stack_slot(StackSlotData::new(
169-
StackSlotKind::ExplicitSlot,
170-
8,
171-
0,
172-
vec![],
173-
));
198+
let ss0 =
199+
func.create_sized_stack_slot(StackSlotData::new(StackSlotKind::ExplicitSlot, 4, 0));
200+
let ss1 =
201+
func.create_sized_stack_slot(StackSlotData::new(StackSlotKind::ExplicitSlot, 8, 0));
174202
assert_eq!(ss0.to_string(), "ss0");
175203
assert_eq!(ss1.to_string(), "ss1");
176204

‎cranelift/codegen/src/isa/aarch64/inst/unwind/systemv.rs‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,7 @@ mod tests {
8585

8686
let mut context = Context::for_function(create_function(
8787
CallConv::SystemV,
88-
Some(StackSlotData::new(
89-
StackSlotKind::ExplicitSlot,
90-
64,
91-
0,
92-
vec![],
93-
)),
88+
Some(StackSlotData::new(StackSlotKind::ExplicitSlot, 64, 0)),
9489
));
9590

9691
let code = context

‎cranelift/codegen/src/isa/riscv64/inst/unwind/systemv.rs‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,7 @@ mod tests {
8080

8181
let mut context = Context::for_function(create_function(
8282
CallConv::SystemV,
83-
Some(StackSlotData::new(
84-
StackSlotKind::ExplicitSlot,
85-
64,
86-
0,
87-
vec![],
88-
)),
83+
Some(StackSlotData::new(StackSlotKind::ExplicitSlot, 64, 0)),
8984
));
9085

9186
let code = context

‎cranelift/codegen/src/isa/s390x/inst/unwind/systemv.rs‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,7 @@ mod tests {
116116

117117
let mut context = Context::for_function(create_function(
118118
CallConv::SystemV,
119-
Some(StackSlotData::new(
120-
StackSlotKind::ExplicitSlot,
121-
64,
122-
0,
123-
vec![],
124-
)),
119+
Some(StackSlotData::new(StackSlotKind::ExplicitSlot, 64, 0)),
125120
));
126121

127122
let code = context
@@ -168,12 +163,7 @@ mod tests {
168163

169164
let mut context = Context::for_function(create_multi_return_function(
170165
CallConv::SystemV,
171-
Some(StackSlotData::new(
172-
StackSlotKind::ExplicitSlot,
173-
64,
174-
0,
175-
vec![],
176-
)),
166+
Some(StackSlotData::new(StackSlotKind::ExplicitSlot, 64, 0)),
177167
));
178168

179169
let code = context

‎cranelift/codegen/src/isa/x64/inst/unwind/systemv.rs‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,7 @@ mod tests {
112112

113113
let mut context = Context::for_function(create_function(
114114
CallConv::SystemV,
115-
Some(StackSlotData::new(
116-
StackSlotKind::ExplicitSlot,
117-
64,
118-
0,
119-
vec![],
120-
)),
115+
Some(StackSlotData::new(StackSlotKind::ExplicitSlot, 64, 0)),
121116
));
122117

123118
let code = context

‎cranelift/codegen/src/machinst/abi.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@
100100
101101
use crate::CodegenError;
102102
use crate::entity::SecondaryMap;
103-
use crate::ir::types::*;
104103
use crate::ir::{ArgumentExtension, ArgumentPurpose, ExceptionTag, Signature};
104+
use crate::ir::{StackSlotKey, types::*};
105105
use crate::isa::TargetIsa;
106106
use crate::settings::ProbestackStrategy;
107107
use crate::{ir, isa};
@@ -1143,7 +1143,7 @@ pub struct Callee<M: ABIMachineSpec> {
11431143
/// Offsets to each sized stackslot.
11441144
sized_stackslots: PrimaryMap<StackSlot, u32>,
11451145
/// Descriptors for sized stackslots.
1146-
sized_stackslot_descriptors: SecondaryMap<StackSlot, Vec<u8>>,
1146+
sized_stackslot_keys: SecondaryMap<StackSlot, Option<StackSlotKey>>,
11471147
/// Total stack size of all stackslots
11481148
stackslots_size: u32,
11491149
/// Stack size to be reserved for outgoing arguments.
@@ -1229,7 +1229,7 @@ impl<M: ABIMachineSpec> Callee<M> {
12291229
// Compute sized stackslot locations and total stackslot size.
12301230
let mut end_offset: u32 = 0;
12311231
let mut sized_stackslots = PrimaryMap::new();
1232-
let mut sized_stackslot_descriptors = SecondaryMap::new();
1232+
let mut sized_stackslot_keys = SecondaryMap::new();
12331233

12341234
for (stackslot, data) in f.sized_stack_slots.iter() {
12351235
// We start our computation possibly unaligned where the previous
@@ -1253,7 +1253,7 @@ impl<M: ABIMachineSpec> Callee<M> {
12531253

12541254
debug_assert_eq!(stackslot.as_u32() as usize, sized_stackslots.len());
12551255
sized_stackslots.push(start_offset);
1256-
sized_stackslot_descriptors[stackslot] = data.descriptor.clone();
1256+
sized_stackslot_keys[stackslot] = data.key;
12571257
}
12581258

12591259
// Compute dynamic stackslot locations and total stackslot size.
@@ -1308,7 +1308,7 @@ impl<M: ABIMachineSpec> Callee<M> {
13081308
dynamic_stackslots,
13091309
dynamic_type_sizes,
13101310
sized_stackslots,
1311-
sized_stackslot_descriptors,
1311+
sized_stackslot_keys,
13121312
stackslots_size,
13131313
outgoing_args_size: 0,
13141314
tail_args_size,
@@ -2416,7 +2416,7 @@ impl<M: ABIMachineSpec> Callee<M> {
24162416
for (slot, storage_area_offset) in &self.sized_stackslots {
24172417
stackslots[slot] = MachBufferStackSlot {
24182418
offset: storage_area_base.checked_add(*storage_area_offset).unwrap(),
2419-
descriptor: self.sized_stackslot_descriptors[slot].clone(),
2419+
key: self.sized_stackslot_keys[slot],
24202420
};
24212421
}
24222422
MachBufferFrameLayout {

‎cranelift/codegen/src/machinst/buffer.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2214,8 +2214,8 @@ pub struct MachBufferStackSlot {
22142214
/// Offset from the bottom of the stack frame.
22152215
pub offset: u32,
22162216

2217-
/// User-provided descriptor bytes for this stack slot.
2218-
pub descriptor: Vec<u8>,
2217+
/// User-provided key to describe this stack slot.
2218+
pub key: Option<ir::StackSlotKey>,
22192219
}
22202220

22212221
/// Debug tags: a sequence of references to a stack slot, or a

‎cranelift/codegen/src/write.rs‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -648,12 +648,7 @@ mod tests {
648648
f.name = UserFuncName::testcase("foo");
649649
assert_eq!(f.to_string(), "function %foo() fast {\n}\n");
650650

651-
f.create_sized_stack_slot(StackSlotData::new(
652-
StackSlotKind::ExplicitSlot,
653-
4,
654-
0,
655-
vec![],
656-
));
651+
f.create_sized_stack_slot(StackSlotData::new(StackSlotKind::ExplicitSlot, 4, 0));
657652
assert_eq!(
658653
f.to_string(),
659654
"function %foo() fast {\n ss0 = explicit_slot 4\n}\n"
@@ -689,12 +684,7 @@ mod tests {
689684
);
690685

691686
let mut f = Function::new();
692-
f.create_sized_stack_slot(StackSlotData::new(
693-
StackSlotKind::ExplicitSlot,
694-
4,
695-
2,
696-
vec![],
697-
));
687+
f.create_sized_stack_slot(StackSlotData::new(StackSlotKind::ExplicitSlot, 4, 2));
698688
assert_eq!(
699689
f.to_string(),
700690
"function u0:0() fast {\n ss0 = explicit_slot 4, align = 4\n}\n"

‎cranelift/filetests/filetests/inline/debug.clif‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ test inline precise-output
22
target x86_64
33

44
function %f0(i32, i32) -> i32 {
5-
ss0 = explicit_slot 64, descriptor = [1, 2, 3, 4]
5+
ss0 = explicit_slot 64, key = 0xfedc_ba98_7654_3210
66
block0(v0: i32, v1: i32):
77
<ss0, 1, 0> sequence_point
88
v2 = iadd v0, v1
@@ -13,16 +13,16 @@ block0(v0: i32, v1: i32):
1313

1414
function %f1() -> i32 {
1515
fn0 = %f0(i32, i32) -> i32
16-
ss0 = explicit_slot 64, descriptor = [5, 6, 7, 8]
16+
ss0 = explicit_slot 64, key = 0x1234_5678_9abc_def0
1717
block0():
1818
v0 = iconst.i32 10
1919
<ss0, 2, 2> v1 = call fn0(v0, v0)
2020
return v1
2121
}
2222

2323
; function %f1() -> i32 fast {
24-
; ss0 = explicit_slot 64, descriptor = [5, 6, 7, 8]
25-
; ss1 = explicit_slot 64, descriptor = [1, 2, 3, 4]
24+
; ss0 = explicit_slot 64, key = 1311768467463790320
25+
; ss1 = explicit_slot 64, key = 18364758544493064720
2626
; sig0 = (i32, i32) -> i32 fast
2727
; fn0 = %f0 sig0
2828
;

0 commit comments

Comments
 (0)