Skip to content

Commit bb153ec

Browse files
committed
Replace inline construction of dead load with call of new dead_load_with_context instruction.
This will give us a convenient place to keep track of dead-load instruction locations and help us reserve the particular registers we need. * Add `mem_flags_aligned_read_only` helper so we can construct aligned-and-read-only `MemFlags`es in ISLE. * Add a `preg_rdi` constructor so we can refer to RDI in ISLE. TODO: Reserve a scratch register to hold the return address.
1 parent 68065d5 commit bb153ec

7 files changed

Lines changed: 67 additions & 9 deletions

File tree

cranelift/codegen/meta/src/shared/instructions.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,36 @@ fn define_control_flow(
380380
.call()
381381
.branches(),
382382
);
383+
384+
ig.push(
385+
Inst::new(
386+
"dead_load_with_context",
387+
r#"
388+
Load 32 bits from memory at ``load_ptr`` while also keeping ``context``
389+
in a fixed register and reserving a second as scratch space.
390+
391+
This is intended for implementing MMU-triggered jumps as in
392+
epoch-interruption-via-mmu, where the load conditionally triggers a
393+
segfault, which hands off control to a signal handler for further
394+
action. The handler has access to ``context`` (typically the
395+
``VMContext``'s ``vm_store_context``) and can use the second
396+
reserved register to store a temp value, as needed on platforms
397+
where signal handlers cannot push stack frames.
398+
"#,
399+
&formats.binary,
400+
)
401+
.operands_in(vec![
402+
Operand::new("load_ptr", iAddr).with_doc("memory location to load from"),
403+
Operand::new("context", iAddr)
404+
.with_doc("arbitrary address-sized context to pass to signal handler"),
405+
])
406+
// Are we a call? stack_switch calls itself one "as it continues execution elsewhere".
407+
// .is_call()
408+
.can_load()
409+
// Don't optimize me out just because I don't def anything. TODO: Can we use side_effects_idempotent()?
410+
.other_side_effects(),
411+
// If `load` is not can_trap(), this isn't either.
412+
);
383413
}
384414

385415
#[inline(never)]

cranelift/codegen/src/isa/x64/inst.isle

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4064,6 +4064,9 @@
40644064
(decl preg_rsp () PReg)
40654065
(extern constructor preg_rsp preg_rsp)
40664066

4067+
(decl preg_rdi () PReg)
4068+
(extern constructor preg_rdi preg_rdi)
4069+
40674070
(decl preg_pinned () PReg)
40684071
(extern constructor preg_pinned preg_pinned)
40694072

cranelift/codegen/src/isa/x64/lower.isle

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3578,6 +3578,17 @@
35783578
(in_payload0 Gpr (put_in_gpr in_payload0)))
35793579
(x64_stack_switch_basic store_context_ptr load_context_ptr in_payload0)))
35803580

3581+
;;;; Rules for `dead_load_with_context` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
3582+
3583+
(rule (lower (dead_load_with_context load_ptr context))
3584+
(let (
3585+
;; Put vmctx into rdi. TODO: Do we have to shove context in a different reg first? The type system seems happy to allow it without.
3586+
(_ SideEffectNoResult (mov_to_preg (preg_rdi) context))
3587+
;; Load from load_ptr to perhaps trigger a trap.
3588+
;; TODO: May be able to change x64_load to higher-level "load", from codegen/meta/src/shared/instructions.rs.
3589+
(_ Gpr (x64_load $I64 (to_amode (mem_flags_aligned_read_only) load_ptr (zero_offset)) (ExtKind.None))))
3590+
(output_none)))
3591+
35813592
;;;; Rules for `get_{frame,stack}_pointer` and `get_return_address` ;;;;;;;;;;;;
35823593

35833594
(rule (lower (get_frame_pointer))

cranelift/codegen/src/isa/x64/lower/isle.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,11 @@ impl Context for IsleContext<'_, '_, MInst, X64Backend> {
658658
regs::rsp().to_real_reg().unwrap().into()
659659
}
660660

661+
#[inline]
662+
fn preg_rdi(&mut self) -> PReg {
663+
regs::rdi().to_real_reg().unwrap().into()
664+
}
665+
661666
#[inline]
662667
fn preg_pinned(&mut self) -> PReg {
663668
regs::pinned_reg().to_real_reg().unwrap().into()

cranelift/codegen/src/isle_prelude.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,11 @@ macro_rules! isle_common_prelude_methods {
638638
MemFlags::trusted()
639639
}
640640

641+
#[inline]
642+
fn mem_flags_aligned_read_only(&mut self) -> MemFlags {
643+
MemFlags::new().with_aligned().with_readonly()
644+
}
645+
641646
#[inline]
642647
fn little_or_native_endian(&mut self, flags: MemFlags) -> Option<MemFlags> {
643648
match flags.explicit_endianness() {

cranelift/codegen/src/prelude.isle

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,10 +304,15 @@
304304

305305
;; `MemFlags::trusted`
306306
(spec (mem_flags_trusted)
307-
(provide (= result #x0003)))
307+
(provide (= result #x0003))) ;; Shouldn't this be 0001?
308308
(decl pure mem_flags_trusted () MemFlags)
309309
(extern constructor mem_flags_trusted mem_flags_trusted)
310310

311+
(spec (mem_flags_aligned_read_only)
312+
(provide (= result #x0003)))
313+
(decl pure mem_flags_aligned_read_only () MemFlags)
314+
(extern constructor mem_flags_aligned_read_only mem_flags_aligned_read_only)
315+
311316
;; Determine if flags specify little- or native-endian.
312317
(decl little_or_native_endian (MemFlags) MemFlags)
313318
(extern extractor little_or_native_endian little_or_native_endian)

crates/cranelift/src/func_environ.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -668,21 +668,20 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
668668
);
669669
builder.def_var(self.epoch_interrupt_page_ptr_var, epoch_interrupt_page_ptr);
670670

671-
Self::epoch_mmu_interruption_check(epoch_interrupt_page_ptr, builder);
671+
self.epoch_mmu_interruption_check(epoch_interrupt_page_ptr, builder);
672672
}
673673

674674
/// Codegens a dead load from the epoch interrupt page, which causes a trap
675675
/// if an interrupt is due.
676676
fn epoch_mmu_interruption_check(
677+
&mut self,
677678
epoch_interrupt_page_ptr: ir::Value,
678679
builder: &mut FunctionBuilder<'_>,
679680
) {
680-
let _ = builder.ins().load(
681-
ir::types::I32, // Arbitrary. Pick whatever works on all ISAs and is fastest.
682-
ir::MemFlags::new().with_aligned().with_readonly(),
683-
epoch_interrupt_page_ptr,
684-
ir::immediates::Offset32::new(0),
685-
);
681+
let vmctx = self.vmctx_val(&mut builder.cursor());
682+
let _ = builder
683+
.ins()
684+
.dead_load_with_context(epoch_interrupt_page_ptr, vmctx);
686685
}
687686

688687
#[cfg(feature = "wmemcheck")]
@@ -3803,7 +3802,7 @@ impl FuncEnvironment<'_> {
38033802
// it's time.
38043803
if self.tunables.epoch_interruption_via_mmu {
38053804
let page_ptr = builder.use_var(self.epoch_interrupt_page_ptr_var);
3806-
Self::epoch_mmu_interruption_check(page_ptr, builder);
3805+
self.epoch_mmu_interruption_check(page_ptr, builder);
38073806
}
38083807

38093808
Ok(())

0 commit comments

Comments
 (0)