Include emergency stack slots when laying out the stack.

Emergency stack slots are a new kind of stack slot added relatively
recently. They need to be allocated a stack offset just like explicit
and spill slots.

Also, make StackSlotData's offset field an Option, to catch problems
like this in the future. Previously the value 0 was used when offsets
weren't assigned yet, however that made it non-obvious when the field
meant "not assigned yet" and when it meant "assigned the value 0".
This commit is contained in:
Dan Gohman
2018-03-03 13:21:10 -08:00
parent a301280d94
commit 13b167770c
9 changed files with 81 additions and 60 deletions

View File

@@ -126,7 +126,7 @@ impl SubTest for TestBinEmit {
// Fix the stack frame layout so we can test spill/fill encodings. // Fix the stack frame layout so we can test spill/fill encodings.
let min_offset = func.stack_slots let min_offset = func.stack_slots
.keys() .keys()
.map(|ss| func.stack_slots[ss].offset) .map(|ss| func.stack_slots[ss].offset.unwrap())
.min(); .min();
func.stack_slots.frame_size = min_offset.map(|off| (-off) as u32); func.stack_slots.frame_size = min_offset.map(|off| (-off) as u32);

View File

@@ -111,7 +111,7 @@ pub struct StackSlotData {
/// ///
/// For `OutgoingArg` stack slots, the offset is relative to the current function's stack /// For `OutgoingArg` stack slots, the offset is relative to the current function's stack
/// pointer immediately before the call. /// pointer immediately before the call.
pub offset: StackOffset, pub offset: Option<StackOffset>,
} }
impl StackSlotData { impl StackSlotData {
@@ -120,7 +120,7 @@ impl StackSlotData {
StackSlotData { StackSlotData {
kind, kind,
size, size,
offset: 0, offset: None,
} }
} }
@@ -138,8 +138,8 @@ impl StackSlotData {
impl fmt::Display for StackSlotData { impl fmt::Display for StackSlotData {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{} {}", self.kind, self.size)?; write!(f, "{} {}", self.kind, self.size)?;
if self.offset != 0 { if let Some(offset) = self.offset {
write!(f, ", offset {}", self.offset)?; write!(f, ", offset {}", offset)?;
} }
Ok(()) Ok(())
} }
@@ -204,7 +204,7 @@ impl StackSlots {
/// Set the offset of a stack slot. /// Set the offset of a stack slot.
pub fn set_offset(&mut self, ss: StackSlot, offset: StackOffset) { pub fn set_offset(&mut self, ss: StackSlot, offset: StackOffset) {
self.slots[ss].offset = offset; self.slots[ss].offset = Some(offset);
} }
/// Get an iterator over all the stack slot keys. /// Get an iterator over all the stack slot keys.
@@ -245,7 +245,7 @@ impl StackSlots {
pub fn make_incoming_arg(&mut self, ty: Type, offset: StackOffset) -> StackSlot { pub fn make_incoming_arg(&mut self, ty: Type, offset: StackOffset) -> StackSlot {
let mut data = StackSlotData::new(StackSlotKind::IncomingArg, ty.bytes()); let mut data = StackSlotData::new(StackSlotKind::IncomingArg, ty.bytes());
assert!(offset <= StackOffset::max_value() - data.size as StackOffset); assert!(offset <= StackOffset::max_value() - data.size as StackOffset);
data.offset = offset; data.offset = Some(offset);
self.push(data) self.push(data)
} }
@@ -261,7 +261,7 @@ impl StackSlots {
// Look for an existing outgoing stack slot with the same offset and size. // Look for an existing outgoing stack slot with the same offset and size.
let inspos = match self.outgoing.binary_search_by_key(&(offset, size), |&ss| { let inspos = match self.outgoing.binary_search_by_key(&(offset, size), |&ss| {
(self[ss].offset, self[ss].size) (self[ss].offset.unwrap(), self[ss].size)
}) { }) {
Ok(idx) => return self.outgoing[idx], Ok(idx) => return self.outgoing[idx],
Err(idx) => idx, Err(idx) => idx,
@@ -270,7 +270,7 @@ impl StackSlots {
// No existing slot found. Make one and insert it into `outgoing`. // No existing slot found. Make one and insert it into `outgoing`.
let mut data = StackSlotData::new(StackSlotKind::OutgoingArg, size); let mut data = StackSlotData::new(StackSlotKind::OutgoingArg, size);
assert!(offset <= StackOffset::max_value() - size as StackOffset); assert!(offset <= StackOffset::max_value() - size as StackOffset);
data.offset = offset; data.offset = Some(offset);
let ss = self.slots.push(data); let ss = self.slots.push(data);
self.outgoing.insert(inspos, ss); self.outgoing.insert(inspos, ss);
ss ss
@@ -344,13 +344,13 @@ mod tests {
let ss1 = sss.get_outgoing_arg(types::I32, 4); let ss1 = sss.get_outgoing_arg(types::I32, 4);
let ss2 = sss.get_outgoing_arg(types::I64, 8); let ss2 = sss.get_outgoing_arg(types::I64, 8);
assert_eq!(sss[ss0].offset, 8); assert_eq!(sss[ss0].offset, Some(8));
assert_eq!(sss[ss0].size, 4); assert_eq!(sss[ss0].size, 4);
assert_eq!(sss[ss1].offset, 4); assert_eq!(sss[ss1].offset, Some(4));
assert_eq!(sss[ss1].size, 4); assert_eq!(sss[ss1].size, 4);
assert_eq!(sss[ss2].offset, 8); assert_eq!(sss[ss2].offset, Some(8));
assert_eq!(sss[ss2].size, 8); assert_eq!(sss[ss2].size, 8);
assert_eq!(sss.get_outgoing_arg(types::I32, 8), ss0); assert_eq!(sss.get_outgoing_arg(types::I32, 8), ss0);

View File

@@ -186,7 +186,7 @@ pub fn spiderwasm_prologue_epilogue(
let bytes = StackSize::from(isa.flags().spiderwasm_prologue_words()) * word_size; let bytes = StackSize::from(isa.flags().spiderwasm_prologue_words()) * word_size;
let mut ss = ir::StackSlotData::new(ir::StackSlotKind::IncomingArg, bytes); let mut ss = ir::StackSlotData::new(ir::StackSlotKind::IncomingArg, bytes);
ss.offset = -(bytes as StackOffset); ss.offset = Some(-(bytes as StackOffset));
func.stack_slots.push(ss); func.stack_slots.push(ss);
layout_stack(&mut func.stack_slots, stack_align)?; layout_stack(&mut func.stack_slots, stack_align)?;
@@ -217,7 +217,7 @@ pub fn native_prologue_epilogue(func: &mut ir::Function, isa: &TargetIsa) -> res
func.create_stack_slot(ir::StackSlotData { func.create_stack_slot(ir::StackSlotData {
kind: ir::StackSlotKind::IncomingArg, kind: ir::StackSlotKind::IncomingArg,
size: csr_stack_size as u32, size: csr_stack_size as u32,
offset: -csr_stack_size, offset: Some(-csr_stack_size),
}); });
let total_stack_size = layout_stack(&mut func.stack_slots, stack_align)? as i32; let total_stack_size = layout_stack(&mut func.stack_slots, stack_align)? as i32;

View File

@@ -251,7 +251,7 @@ pub trait TargetIsa: fmt::Display {
if func.signature.call_conv == ir::CallConv::SpiderWASM { if func.signature.call_conv == ir::CallConv::SpiderWASM {
let bytes = StackSize::from(self.flags().spiderwasm_prologue_words()) * word_size; let bytes = StackSize::from(self.flags().spiderwasm_prologue_words()) * word_size;
let mut ss = ir::StackSlotData::new(ir::StackSlotKind::IncomingArg, bytes); let mut ss = ir::StackSlotData::new(ir::StackSlotKind::IncomingArg, bytes);
ss.offset = -(bytes as StackOffset); ss.offset = Some(-(bytes as StackOffset));
func.stack_slots.push(ss); func.stack_slots.push(ss);
} }

View File

@@ -41,12 +41,12 @@ impl StackRef {
let slot = &frame[ss]; let slot = &frame[ss];
let offset = if slot.kind == StackSlotKind::OutgoingArg { let offset = if slot.kind == StackSlotKind::OutgoingArg {
// Outgoing argument slots have offsets relative to our stack pointer. // Outgoing argument slots have offsets relative to our stack pointer.
slot.offset slot.offset.unwrap()
} else { } else {
// All other slots have offsets relative to our caller's stack frame. // All other slots have offsets relative to our caller's stack frame.
// Offset where SP is pointing. (All ISAs have stacks growing downwards.) // Offset where SP is pointing. (All ISAs have stacks growing downwards.)
let sp_offset = -(size as StackOffset); let sp_offset = -(size as StackOffset);
slot.offset - sp_offset slot.offset.unwrap() - sp_offset
}; };
StackRef { StackRef {
base: StackBase::SP, base: StackBase::SP,

View File

@@ -48,12 +48,13 @@ pub fn layout_stack(frame: &mut StackSlots, alignment: StackSize) -> Result<Stac
match slot.kind { match slot.kind {
StackSlotKind::IncomingArg => { StackSlotKind::IncomingArg => {
incoming_min = min(incoming_min, slot.offset); incoming_min = min(incoming_min, slot.offset.unwrap());
} }
StackSlotKind::OutgoingArg => { StackSlotKind::OutgoingArg => {
let offset = slot.offset.checked_add(slot.size as StackOffset).ok_or( let offset = slot.offset
CtonError::ImplLimitExceeded, .unwrap()
)?; .checked_add(slot.size as StackOffset)
.ok_or(CtonError::ImplLimitExceeded)?;
outgoing_max = max(outgoing_max, offset); outgoing_max = max(outgoing_max, offset);
} }
StackSlotKind::SpillSlot | StackSlotKind::SpillSlot |
@@ -77,12 +78,14 @@ pub fn layout_stack(frame: &mut StackSlots, alignment: StackSize) -> Result<Stac
// Pick out explicit and spill slots with exact alignment `min_align`. // Pick out explicit and spill slots with exact alignment `min_align`.
match slot.kind { match slot.kind {
StackSlotKind::SpillSlot | StackSlotKind::SpillSlot |
StackSlotKind::ExplicitSlot => { StackSlotKind::ExplicitSlot |
StackSlotKind::EmergencySlot => {
if slot.alignment(alignment) != min_align { if slot.alignment(alignment) != min_align {
continue; continue;
} }
} }
_ => continue, StackSlotKind::IncomingArg |
StackSlotKind::OutgoingArg => continue,
} }
offset = offset.checked_sub(slot.size as StackOffset).ok_or( offset = offset.checked_sub(slot.size as StackOffset).ok_or(
@@ -111,7 +114,7 @@ pub fn layout_stack(frame: &mut StackSlots, alignment: StackSize) -> Result<Stac
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use ir::StackSlots; use ir::{StackSlots, StackSlotData, StackSlotKind};
use ir::types; use ir::types;
use super::layout_stack; use super::layout_stack;
use ir::stackslot::StackOffset; use ir::stackslot::StackOffset;
@@ -131,64 +134,82 @@ mod tests {
assert_eq!(layout_stack(sss, 1), Ok(0)); assert_eq!(layout_stack(sss, 1), Ok(0));
assert_eq!(layout_stack(sss, 16), Ok(0)); assert_eq!(layout_stack(sss, 16), Ok(0));
assert_eq!(sss[in0].offset, 0); assert_eq!(sss[in0].offset, Some(0));
assert_eq!(sss[in1].offset, 8); assert_eq!(sss[in1].offset, Some(8));
// Add some spill slots. // Add some spill slots.
let ss0 = sss.make_spill_slot(types::I64); let ss0 = sss.make_spill_slot(types::I64);
let ss1 = sss.make_spill_slot(types::I32); let ss1 = sss.make_spill_slot(types::I32);
assert_eq!(layout_stack(sss, 1), Ok(12)); assert_eq!(layout_stack(sss, 1), Ok(12));
assert_eq!(sss[in0].offset, 0); assert_eq!(sss[in0].offset, Some(0));
assert_eq!(sss[in1].offset, 8); assert_eq!(sss[in1].offset, Some(8));
assert_eq!(sss[ss0].offset, -8); assert_eq!(sss[ss0].offset, Some(-8));
assert_eq!(sss[ss1].offset, -12); assert_eq!(sss[ss1].offset, Some(-12));
assert_eq!(layout_stack(sss, 16), Ok(16)); assert_eq!(layout_stack(sss, 16), Ok(16));
assert_eq!(sss[in0].offset, 0); assert_eq!(sss[in0].offset, Some(0));
assert_eq!(sss[in1].offset, 8); assert_eq!(sss[in1].offset, Some(8));
assert_eq!(sss[ss0].offset, -16); assert_eq!(sss[ss0].offset, Some(-16));
assert_eq!(sss[ss1].offset, -4); assert_eq!(sss[ss1].offset, Some(-4));
// An incoming argument with negative offset counts towards the total frame size, but it // An incoming argument with negative offset counts towards the total frame size, but it
// should still pack nicely with the spill slots. // should still pack nicely with the spill slots.
let in2 = sss.make_incoming_arg(types::I32, -4); let in2 = sss.make_incoming_arg(types::I32, -4);
assert_eq!(layout_stack(sss, 1), Ok(16)); assert_eq!(layout_stack(sss, 1), Ok(16));
assert_eq!(sss[in0].offset, 0); assert_eq!(sss[in0].offset, Some(0));
assert_eq!(sss[in1].offset, 8); assert_eq!(sss[in1].offset, Some(8));
assert_eq!(sss[in2].offset, -4); assert_eq!(sss[in2].offset, Some(-4));
assert_eq!(sss[ss0].offset, -12); assert_eq!(sss[ss0].offset, Some(-12));
assert_eq!(sss[ss1].offset, -16); assert_eq!(sss[ss1].offset, Some(-16));
assert_eq!(layout_stack(sss, 16), Ok(16)); assert_eq!(layout_stack(sss, 16), Ok(16));
assert_eq!(sss[in0].offset, 0); assert_eq!(sss[in0].offset, Some(0));
assert_eq!(sss[in1].offset, 8); assert_eq!(sss[in1].offset, Some(8));
assert_eq!(sss[in2].offset, -4); assert_eq!(sss[in2].offset, Some(-4));
assert_eq!(sss[ss0].offset, -16); assert_eq!(sss[ss0].offset, Some(-16));
assert_eq!(sss[ss1].offset, -8); assert_eq!(sss[ss1].offset, Some(-8));
// Finally, make sure there is room for the outgoing args. // Finally, make sure there is room for the outgoing args.
let out0 = sss.get_outgoing_arg(types::I32, 0); let out0 = sss.get_outgoing_arg(types::I32, 0);
assert_eq!(layout_stack(sss, 1), Ok(20)); assert_eq!(layout_stack(sss, 1), Ok(20));
assert_eq!(sss[in0].offset, 0); assert_eq!(sss[in0].offset, Some(0));
assert_eq!(sss[in1].offset, 8); assert_eq!(sss[in1].offset, Some(8));
assert_eq!(sss[in2].offset, -4); assert_eq!(sss[in2].offset, Some(-4));
assert_eq!(sss[ss0].offset, -12); assert_eq!(sss[ss0].offset, Some(-12));
assert_eq!(sss[ss1].offset, -16); assert_eq!(sss[ss1].offset, Some(-16));
assert_eq!(sss[out0].offset, 0); assert_eq!(sss[out0].offset, Some(0));
assert_eq!(layout_stack(sss, 16), Ok(32)); assert_eq!(layout_stack(sss, 16), Ok(32));
assert_eq!(sss[in0].offset, 0); assert_eq!(sss[in0].offset, Some(0));
assert_eq!(sss[in1].offset, 8); assert_eq!(sss[in1].offset, Some(8));
assert_eq!(sss[in2].offset, -4); assert_eq!(sss[in2].offset, Some(-4));
assert_eq!(sss[ss0].offset, -16); assert_eq!(sss[ss0].offset, Some(-16));
assert_eq!(sss[ss1].offset, -8); assert_eq!(sss[ss1].offset, Some(-8));
assert_eq!(sss[out0].offset, 0); assert_eq!(sss[out0].offset, Some(0));
// Also test that an unsupported offset is rejected. // Also test that an unsupported offset is rejected.
sss.get_outgoing_arg(types::I8, StackOffset::max_value() - 1); sss.get_outgoing_arg(types::I8, StackOffset::max_value() - 1);
assert_eq!(layout_stack(sss, 1), Err(CtonError::ImplLimitExceeded)); assert_eq!(layout_stack(sss, 1), Err(CtonError::ImplLimitExceeded));
} }
#[test]
fn slot_kinds() {
let sss = &mut StackSlots::new();
// Add some slots of various kinds.
let ss0 = sss.make_spill_slot(types::I32);
let ss1 = sss.push(StackSlotData::new(
StackSlotKind::ExplicitSlot,
types::I32.bytes(),
));
let ss2 = sss.get_emergency_slot(types::I32, &[]);
assert_eq!(layout_stack(sss, 1), Ok(12));
assert_eq!(sss[ss0].offset, Some(-4));
assert_eq!(sss[ss1].offset, Some(-8));
assert_eq!(sss[ss2].offset, Some(-12));
}
} }

View File

@@ -207,14 +207,14 @@ impl<'a> LocationVerifier<'a> {
slot.kind slot.kind
); );
} }
if slot.offset != offset { if slot.offset.unwrap() != offset {
return err!( return err!(
inst, inst,
"ABI expects {} at stack offset {}, but {} is at {}", "ABI expects {} at stack offset {}, but {} is at {}",
value, value,
offset, offset,
ss, ss,
slot.offset slot.offset.unwrap()
); );
} }
} else { } else {

View File

@@ -808,7 +808,7 @@ impl<'a> Verifier<'a> {
slot slot
); );
} }
if slot.offset != offset { if slot.offset != Some(offset) {
return err!( return err!(
inst, inst,
"Outgoing stack argument {} should have offset {}: {} = {}", "Outgoing stack argument {} should have offset {}: {} = {}",

View File

@@ -1070,7 +1070,7 @@ impl<'a> Parser<'a> {
// Take additional options. // Take additional options.
while self.optional(Token::Comma) { while self.optional(Token::Comma) {
match self.match_any_identifier("expected stack slot flags")? { match self.match_any_identifier("expected stack slot flags")? {
"offset" => data.offset = self.match_imm32("expected byte offset")?, "offset" => data.offset = Some(self.match_imm32("expected byte offset")?),
other => return err!(self.loc, "Unknown stack slot flag '{}'", other), other => return err!(self.loc, "Unknown stack slot flag '{}'", other),
} }
} }