Refactor dead_load_with_context so we can capture pre- and post-instruction offsets.

x64 movq instructions are usually 3 bytes but grow to 4 when the source reg is R12 (or RSP). We'll need to know which at each callsite so we can both identify MMU-driven epoch interrupts (beginning offset) and resume at the right location (ending offset).

Also...
* Store the dead load's result to r10, which we're clobbering anyway, to save a reg.
* Remove the x64_dead_load_with_context layer.

Author: erikrose

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

@@ -194,7 +194,8 @@
                     (offset i64)
                     (distance RelocDistance))
 
-       (DeadLoadWithContext (load_ptr Gpr)
+       (DeadLoadWithContext (dst WritableGpr)
+                            (load_ptr Gpr)
                             (context Gpr))
 
        ;; =========================================
@@ -1345,12 +1346,6 @@
 (rule (return_call_unknown info)
       (SideEffectNoResult.Inst (MInst.ReturnCallUnknown info)))
 
-;; Helper for creating `DeadLoadWithContext` instructions.
-(decl x64_dead_load_with_context (Gpr Gpr) SideEffectNoResult)
-(rule (x64_dead_load_with_context load_ptr context)
-      (SideEffectNoResult.Inst (MInst.DeadLoadWithContext load_ptr
-                                                          context)))
-
 ;;;; Helpers for emitting stack switches ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 (decl x64_stack_switch_basic (Gpr Gpr Gpr) Gpr)

cranelift/codegen/src/isa/x64/inst/emit.rs

@@ -645,10 +645,19 @@ pub(crate) fn emit(
             sink.bind_label(resume, state.ctrl_plane_mut());
         }
 
-        Inst::DeadLoadWithContext { .. } => {
-            // The ISLE has already emitted the dead load. Put the address of
-            // this instruction aside so we can later distinguish whether a
-            // segfault is its fault.
+        Inst::DeadLoadWithContext { dst, load_ptr, .. } => {
+            let start = sink.cur_offset();
+
+            // Since we're clobbering r10 anyway to store the original return
+            // address, we also use it as a destination for the dead load rather
+            // than sucking up another reg:
+            let load_ptr_addr = SyntheticAmode::real(Amode::imm_reg(0, **load_ptr));
+            asm::inst::movq_rm::new(*dst, load_ptr_addr).emit(sink, info, state);
+
+            let end = sink.cur_offset();
+
+            // Put the address of this instruction aside so we can later
+            // distinguish whether a segfault is its fault.
             sink.add_epoch_check();
         }
 

cranelift/codegen/src/isa/x64/inst/mod.rs

@@ -1,7 +1,6 @@
 //! This module defines x86_64-specific machine instruction types.
 
 pub use emit_state::EmitState;
-use regalloc2::PRegSet;
 
 use crate::binemit::{Addend, CodeOffset, Reloc};
 use crate::ir::{ExternalName, LibCall, TrapCode, Type, types};
@@ -673,10 +672,15 @@ impl PrettyPrint for Inst {
                 )
             }
 
-            Inst::DeadLoadWithContext { load_ptr, context } => {
+            Inst::DeadLoadWithContext {
+                dst,
+                load_ptr,
+                context,
+            } => {
+                let dst = pretty_print_reg(*dst.to_reg(), 8);
                 let load_ptr = pretty_print_reg(**load_ptr, 8);
                 let context = pretty_print_reg(**context, 8);
-                format!("dead_load_with_context {load_ptr}, {context}")
+                format!("dead_load_with_context {dst}, {load_ptr}, {context}")
             }
 
             Inst::JmpKnown { dst } => {
@@ -1053,7 +1057,11 @@ fn x64_get_operands(inst: &mut Inst, collector: &mut impl OperandVisitor) {
             collector.reg_clobbers(clobbers);
         }
 
-        Inst::DeadLoadWithContext { load_ptr, context } => {
+        Inst::DeadLoadWithContext {
+            dst,
+            load_ptr,
+            context,
+        } => {
             // load_ptr is an input param.
             collector.reg_use(load_ptr);
             // Demand context (vmctx) go into RDI.
@@ -1063,7 +1071,10 @@ fn x64_get_operands(inst: &mut Inst, collector: &mut impl OperandVisitor) {
             // address (which we're overwriting with that of the epoch-ending
             // stub). Picking r10 because it's caller-saved but otherwise
             // arbitrarily.
-            collector.reg_clobbers(PRegSet::empty().with(regs::gpr_preg(asm::gpr::enc::R10)));
+            //
+            // Also def it so we can use it as the destination of the dead load
+            // rather than consuming another arbitrary reg.
+            collector.reg_fixed_def(dst, regs::r10());
         }
 
         Inst::ReturnCallKnown { info } => {

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

@@ -3581,18 +3581,13 @@
 ;;;; Rules for `dead_load_with_context` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 (rule (lower (dead_load_with_context load_ptr context))
-      (let (
-            ;; 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.
-            ;; Actually, I suspect we don't even need to do this move; because we use reg_fixed_use() to constrain the `context` arg of DeadLoadWithContext to RDI, I hypothesize regalloc will insert any MOV needed to make that so. It is, after all, in the business of inserting MOVs for spills.
-            ;; (_ SideEffectNoResult (mov_to_preg (preg_rdi) context))
-
-            ;; Load from load_ptr to perhaps trigger a trap.
-            ;; TODO: May be able to change x64_load to higher-level "load", from codegen/meta/src/shared/instructions.rs.
-            (_ Gpr (x64_load $I64 (to_amode (mem_flags_aligned_read_only)
+      (let ((load_ptr Gpr (put_in_gpr load_ptr))
+            (context Gpr (put_in_gpr context))
+            (dst WritableGpr (temp_writable_gpr))
+            (_ Unit (emit_side_effect (SideEffectNoResult.Inst
+                  (MInst.DeadLoadWithContext dst
                                              load_ptr
-                                             (zero_offset))
-                                  (ExtKind.None)))
-            (_ Unit (emit_side_effect (x64_dead_load_with_context load_ptr context))))
+                                             context)))))
        (output_none)))
 
 ;;;; Rules for `get_{frame,stack}_pointer` and `get_return_address` ;;;;;;;;;;;;

tests/disas/epoch-interruption-mmu-compile-loop.wat

@@ -14,6 +14,6 @@
 ;;       movq    %rsp, %rbp
 ;;       movq    8(%rdi), %r8
 ;;       movq    0x10(%r8), %r8
-;;       movq    (%r8), %r9
+;;       movq    (%r8), %r10
 ;;       movq    (%r8), %r10
 ;;       jmp     0xf

tests/disas/epoch-interruption-mmu-compile.wat

@@ -11,7 +11,7 @@
 ;;       movq    %rsp, %rbp
 ;;       movq    8(%rdi), %rdx
 ;;       movq    0x10(%rdx), %rdx
-;;       movq    (%rdx), %r8
+;;       movq    (%rdx), %r10
 ;;       movq    %rbp, %rsp
 ;;       popq    %rbp
 ;;       retq