Skip to content

Commit

Permalink
x64 and aarch64: carry MemFlags on loads/stores; don't emit trap info…
Browse files Browse the repository at this point in the history 
… unless an op can trap.

This end result was previously enacted by carrying a `SourceLoc` on
every load/store, which was somewhat cumbersome, and only indirectly
encoded metadata about a memory reference (can it trap) by its presence
or absence. We have a type for this -- `MemFlags` -- that tells us
everything we might want to know about a load or store, and we should
plumb it through to code emission instead.

This PR attaches a `MemFlags` to an `Amode` on x64, and puts it on load
and store `Inst` variants on aarch64. These two choices seem to factor
things out in the nicest way: there are relatively few load/store insts
on aarch64 but many addressing modes, while the opposite is true on x64.
  • Loading branch information
@cfallin
cfallin committed Nov 17, 2020
1 parent b97f07b commit 1b16229
Show file tree
Hide file tree
Showing 11 changed files with 340 additions and 89 deletions.
16 changes: 12 additions & 4 deletions cranelift/codegen/src/isa/aarch64/abi.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use crate::ir;
use crate::ir::types;
use crate::ir::types::*;
use crate::ir::MemFlags;
use crate::isa;
use crate::isa::aarch64::{inst::EmitState, inst::*};
use crate::machinst::*;
Expand Down Expand Up @@ -312,11 +313,11 @@ impl ABIMachineSpec for AArch64MachineDeps {
}

fn gen_load_stack(mem: StackAMode, into_reg: Writable<Reg>, ty: Type) -> Inst {
Inst::gen_load(into_reg, mem.into(), ty)
Inst::gen_load(into_reg, mem.into(), ty, MemFlags::trusted())
}

fn gen_store_stack(mem: StackAMode, from_reg: Reg, ty: Type) -> Inst {
Inst::gen_store(mem.into(), from_reg, ty)
Inst::gen_store(mem.into(), from_reg, ty, MemFlags::trusted())
}

fn gen_move(to_reg: Writable<Reg>, from_reg: Reg, ty: Type) -> Inst {
Expand Down Expand Up @@ -402,12 +403,12 @@ impl ABIMachineSpec for AArch64MachineDeps {

fn gen_load_base_offset(into_reg: Writable<Reg>, base: Reg, offset: i32, ty: Type) -> Inst {
let mem = AMode::RegOffset(base, offset as i64, ty);
Inst::gen_load(into_reg, mem, ty)
Inst::gen_load(into_reg, mem, ty, MemFlags::trusted())
}

fn gen_store_base_offset(base: Reg, offset: i32, from_reg: Reg, ty: Type) -> Inst {
let mem = AMode::RegOffset(base, offset as i64, ty);
Inst::gen_store(mem, from_reg, ty)
Inst::gen_store(mem, from_reg, ty, MemFlags::trusted())
}

fn gen_sp_reg_adjust(amount: i32) -> SmallVec<[Inst; 2]> {
Expand Down Expand Up @@ -464,6 +465,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
writable_stack_reg(),
SImm7Scaled::maybe_from_i64(-16, types::I64).unwrap(),
),
flags: MemFlags::trusted(),
});
// mov fp (x29), sp. This uses the ADDI rd, rs, 0 form of `MOV` because
// the usual encoding (`ORR`) does not work with SP.
Expand Down Expand Up @@ -500,6 +502,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
writable_stack_reg(),
SImm7Scaled::maybe_from_i64(16, types::I64).unwrap(),
),
flags: MemFlags::trusted(),
});

insts
Expand Down Expand Up @@ -542,6 +545,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
stack_reg(),
SImm7Scaled::maybe_from_i64((i * 16) as i64, types::I64).unwrap(),
),
flags: MemFlags::trusted(),
});
}

Expand All @@ -553,6 +557,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
stack_reg(),
SImm9::maybe_from_i64((vec_offset + (i * 16)) as i64).unwrap(),
),
flags: MemFlags::trusted(),
});
}

Expand Down Expand Up @@ -591,6 +596,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
stack_reg(),
SImm7Scaled::maybe_from_i64((i * 16) as i64, types::I64).unwrap(),
),
flags: MemFlags::trusted(),
});
}

Expand All @@ -601,6 +607,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
stack_reg(),
SImm9::maybe_from_i64(((i * 16) + int_save_bytes) as i64).unwrap(),
),
flags: MemFlags::trusted(),
});
}

Expand All @@ -621,6 +628,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
writable_xreg(BALDRDASH_TLS_REG),
AMode::UnsignedOffset(fp_reg(), UImm12Scaled::maybe_from_i64(off, I64).unwrap()),
I64,
MemFlags::trusted(),
));
}

Expand Down
93 changes: 61 additions & 32 deletions cranelift/codegen/src/isa/aarch64/inst/emit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use crate::binemit::{CodeOffset, Reloc, StackMap};
use crate::ir::constant::ConstantData;
use crate::ir::types::*;
use crate::ir::TrapCode;
use crate::ir::{MemFlags, TrapCode};
use crate::isa::aarch64::inst::*;
use crate::machinst::ty_bits;

Expand Down Expand Up @@ -741,16 +741,18 @@ impl MachInstEmit for Inst {
sink.put4(enc_bit_rr(size, op1, op2, rn, rd))
}

&Inst::ULoad8 { rd, ref mem }
| &Inst::SLoad8 { rd, ref mem }
| &Inst::ULoad16 { rd, ref mem }
| &Inst::SLoad16 { rd, ref mem }
| &Inst::ULoad32 { rd, ref mem }
| &Inst::SLoad32 { rd, ref mem }
| &Inst::ULoad64 { rd, ref mem, .. }
| &Inst::FpuLoad32 { rd, ref mem }
| &Inst::FpuLoad64 { rd, ref mem }
| &Inst::FpuLoad128 { rd, ref mem } => {
&Inst::ULoad8 { rd, ref mem, flags }
| &Inst::SLoad8 { rd, ref mem, flags }
| &Inst::ULoad16 { rd, ref mem, flags }
| &Inst::SLoad16 { rd, ref mem, flags }
| &Inst::ULoad32 { rd, ref mem, flags }
| &Inst::SLoad32 { rd, ref mem, flags }
| &Inst::ULoad64 {
rd, ref mem, flags, ..
}
| &Inst::FpuLoad32 { rd, ref mem, flags }
| &Inst::FpuLoad64 { rd, ref mem, flags }
| &Inst::FpuLoad128 { rd, ref mem, flags } => {
let (mem_insts, mem) = mem_finalize(sink.cur_offset(), mem, state);

for inst in mem_insts.into_iter() {
Expand Down Expand Up @@ -778,7 +780,7 @@ impl MachInstEmit for Inst {
};

let srcloc = state.cur_srcloc();
if srcloc != SourceLoc::default() {
if srcloc != SourceLoc::default() && !flags.notrap() {
// Register the offset at which the actual load instruction starts.
sink.add_trap(srcloc, TrapCode::HeapOutOfBounds);
}
Expand Down Expand Up @@ -861,13 +863,13 @@ impl MachInstEmit for Inst {
}
}

&Inst::Store8 { rd, ref mem }
| &Inst::Store16 { rd, ref mem }
| &Inst::Store32 { rd, ref mem }
| &Inst::Store64 { rd, ref mem, .. }
| &Inst::FpuStore32 { rd, ref mem }
| &Inst::FpuStore64 { rd, ref mem }
| &Inst::FpuStore128 { rd, ref mem } => {
&Inst::Store8 { rd, ref mem, flags }
| &Inst::Store16 { rd, ref mem, flags }
| &Inst::Store32 { rd, ref mem, flags }
| &Inst::Store64 { rd, ref mem, flags }
| &Inst::FpuStore32 { rd, ref mem, flags }
| &Inst::FpuStore64 { rd, ref mem, flags }
| &Inst::FpuStore128 { rd, ref mem, flags } => {
let (mem_insts, mem) = mem_finalize(sink.cur_offset(), mem, state);

for inst in mem_insts.into_iter() {
Expand All @@ -886,7 +888,7 @@ impl MachInstEmit for Inst {
};

let srcloc = state.cur_srcloc();
if srcloc != SourceLoc::default() {
if srcloc != SourceLoc::default() && !flags.notrap() {
// Register the offset at which the actual load instruction starts.
sink.add_trap(srcloc, TrapCode::HeapOutOfBounds);
}
Expand Down Expand Up @@ -943,21 +945,44 @@ impl MachInstEmit for Inst {
}
}

&Inst::StoreP64 { rt, rt2, ref mem } => match mem {
&PairAMode::SignedOffset(reg, simm7) => {
assert_eq!(simm7.scale_ty, I64);
sink.put4(enc_ldst_pair(0b1010100100, simm7, reg, rt, rt2));
&Inst::StoreP64 {
rt,
rt2,
ref mem,
flags,
} => {
let srcloc = state.cur_srcloc();
if srcloc != SourceLoc::default() && !flags.notrap() {
// Register the offset at which the actual load instruction starts.
sink.add_trap(srcloc, TrapCode::HeapOutOfBounds);
}
&PairAMode::PreIndexed(reg, simm7) => {
assert_eq!(simm7.scale_ty, I64);
sink.put4(enc_ldst_pair(0b1010100110, simm7, reg.to_reg(), rt, rt2));
match mem {
&PairAMode::SignedOffset(reg, simm7) => {
assert_eq!(simm7.scale_ty, I64);
sink.put4(enc_ldst_pair(0b1010100100, simm7, reg, rt, rt2));
}
&PairAMode::PreIndexed(reg, simm7) => {
assert_eq!(simm7.scale_ty, I64);
sink.put4(enc_ldst_pair(0b1010100110, simm7, reg.to_reg(), rt, rt2));
}
&PairAMode::PostIndexed(reg, simm7) => {
assert_eq!(simm7.scale_ty, I64);
sink.put4(enc_ldst_pair(0b1010100010, simm7, reg.to_reg(), rt, rt2));
}
}
&PairAMode::PostIndexed(reg, simm7) => {
assert_eq!(simm7.scale_ty, I64);
sink.put4(enc_ldst_pair(0b1010100010, simm7, reg.to_reg(), rt, rt2));
}
&Inst::LoadP64 {
rt,
rt2,
ref mem,
flags,
} => {
let srcloc = state.cur_srcloc();
if srcloc != SourceLoc::default() && !flags.notrap() {
// Register the offset at which the actual load instruction starts.
sink.add_trap(srcloc, TrapCode::HeapOutOfBounds);
}
},
&Inst::LoadP64 { rt, rt2, ref mem } => {

let rt = rt.to_reg();
let rt2 = rt2.to_reg();
match mem {
Expand Down Expand Up @@ -1546,6 +1571,7 @@ impl MachInstEmit for Inst {
let inst = Inst::FpuLoad64 {
rd,
mem: AMode::Label(MemLabel::PCRel(8)),
flags: MemFlags::trusted(),
};
inst.emit(sink, emit_info, state);
let inst = Inst::Jump {
Expand All @@ -1558,6 +1584,7 @@ impl MachInstEmit for Inst {
let inst = Inst::FpuLoad128 {
rd,
mem: AMode::Label(MemLabel::PCRel(8)),
flags: MemFlags::trusted(),
};
inst.emit(sink, emit_info, state);
let inst = Inst::Jump {
Expand Down Expand Up @@ -2169,6 +2196,7 @@ impl MachInstEmit for Inst {
I32,
ExtendOp::UXTW,
),
flags: MemFlags::trusted(),
};
inst.emit(sink, emit_info, state);
// Add base of jump table to jump-table-sourced block offset
Expand Down Expand Up @@ -2215,6 +2243,7 @@ impl MachInstEmit for Inst {
let inst = Inst::ULoad64 {
rd,
mem: AMode::Label(MemLabel::PCRel(8)),
flags: MemFlags::trusted(),
};
inst.emit(sink, emit_info, state);
let inst = Inst::Jump {
Expand Down
Loading

0 comments on commit 1b16229

Please sign in to comment.