Two JIT admission gates that answered wider than their evidence

majit/majit-backend-cranelift/src/compiler.rs

@@ -10509,9 +10509,34 @@ impl CraneliftBackend {
                     // earlier LABEL. Re-materialize it from the forwarded root
                     // slot at this header so the fall-through transfer can pass
                     // it onward without restoring a loop phi.
+                    //
+                    // That later LABEL is the only reader of the SSA variable.
+                    // The loop body never `use_var`s a demoted ref — the reason
+                    // `spill_ref_roots` and `reload_ref_roots` both skip one —
+                    // a guard exit reaches the value through
+                    // `demoted_failarg_slots`, and a JUMP filters demoted
+                    // positions out of its args. With no later LABEL demoting
+                    // the same raw the load has no use, and a load is never
+                    // dead code to Cranelift: `MemFlags::trusted()` is not
+                    // `readonly`, so the egraph keeps it, and it would sit in
+                    // the header on every iteration of the loop.
                     if let Some(positions) = demoted_ref_positions_by_label.get(&op_idx) {
-                        let cur_jf = builder.ins().get_pinned_reg(ptr_type);
+                        let mut cur_jf = None;
                         for &(_, raw, ofs) in positions {
+                            let passed_on =
+                                demoted_ref_positions_by_label
+                                    .iter()
+                                    .any(|(&later_idx, later)| {
+                                        later_idx > op_idx
+                                            && later
+                                                .iter()
+                                                .any(|&(_, later_raw, _)| later_raw == raw)
+                                    });
+                            if !passed_on {
+                                continue;
+                            }
+                            let cur_jf = *cur_jf
+                                .get_or_insert_with(|| builder.ins().get_pinned_reg(ptr_type));
                             let value = builder.ins().load(
                                 cl_types::I64,
                                 MemFlagsData::trusted(),

majit/majit-metainterp/src/blackhole.rs

@@ -2645,7 +2645,7 @@ pub fn run_forever(
     bh: BlackholeInterpreter,
     current_exc: i64,
 ) -> JitException {
-    run_forever_with_portal(builder, bh, current_exc, None, None, None)
+    run_forever_with_portal(builder, bh, current_exc, None, None, None, None)
 }
 
 /// blackhole.py:1752 _run_forever with optional portal runner callback.
@@ -2668,6 +2668,7 @@ pub fn run_forever_with_portal(
     mut current_exc: i64,
     portal_runner: Option<&dyn Fn(&JitException) -> Result<(BhReturnType, i64), JitException>>,
     on_enter_level: Option<&dyn Fn(i64)>,
+    on_leave_level: Option<&dyn Fn(i64)>,
     mut terminal_out: Option<&mut Option<BlackholeTerminalImage>>,
 ) -> JitException {
     loop {
@@ -2717,6 +2718,18 @@ pub fn run_forever_with_portal(
 
         // blackhole.py:1759
         let next = bh.nextblackholeinterp.take();
+        // `pyopcode.py:239-241 RETURN_VALUE` (`frame_finished_execution = True`)
+        // and `pyopcode.py:184 handle_operation_error` (the same store on the
+        // no-handler propagation): the level reached here has returned to its
+        // caller by one of those two routes, so its frame's execution is over.
+        // Threaded from the interpreter side for the same reason as
+        // `on_enter_level` — the transition is a property of the embedder's
+        // frame object, which majit-metainterp cannot name.  The bottommost
+        // level never arrives: it leaves through `handle_jitexception`'s
+        // propagating arm, which returns above.
+        if let Some(on_leave_level) = on_leave_level {
+            on_leave_level(bh.virtualizable_ptr);
+        }
         builder.release_interp(bh);
         // blackhole.py:1760
         // RPython: blackholeinterp = blackholeinterp.nextblackholeinterp
@@ -2747,6 +2760,17 @@ pub struct PyjitplBlackholeFrameConfig<'a> {
     /// the resumed frame chain.  Threaded from the interpreter side because
     /// majit-metainterp cannot reference `ExecutionContext`.
     pub on_enter_level: Option<&'a dyn Fn(i64)>,
+    /// The `frame_finished_execution` store `pyopcode.py:239-241 RETURN_VALUE`
+    /// and `pyopcode.py:184 handle_operation_error` perform before leaving a
+    /// frame.  Threaded from the interpreter side for the same reason as
+    /// [`Self::on_enter_level`]; called once per level that returns to its
+    /// caller, with that level's `virtualizable_ptr`.
+    ///
+    /// Set it only alongside [`Self::per_frame`], which is what makes that
+    /// pointer name the level's OWN frame.  Without it every level shares the
+    /// portal's virtualizable, and a nested level would hand back the frame
+    /// ABOVE it — marking a caller that is still running as finished.
+    pub on_leave_level: Option<&'a dyn Fn(i64)>,
 }
 
 pub fn convert_and_run_from_pyjitpl(
@@ -2761,6 +2785,7 @@ pub fn convert_and_run_from_pyjitpl(
     let mut next_bh: Option<Box<BlackholeInterpreter>> = None;
     let roots_depth = majit_gc::shadow_stack::resume_ref_roots_depth();
     let on_enter_level = config.as_ref().and_then(|config| config.on_enter_level);
+    let on_leave_level = config.as_ref().and_then(|config| config.on_leave_level);
 
     for (frame_index, frame) in framestack.frames.iter().enumerate() {
         let mut cur_bh = builder.acquire_interp();
@@ -2807,6 +2832,7 @@ pub fn convert_and_run_from_pyjitpl(
         current_exc,
         None,
         on_enter_level,
+        on_leave_level,
         terminal_out,
     );
     majit_gc::shadow_stack::pop_resume_ref_roots_to(roots_depth);
@@ -4222,6 +4248,7 @@ mod tests {
                 Some(&portal_runner),
                 None,
                 None,
+                None,
             );
             assert!(
                 matches!(

majit/majit-metainterp/src/jitdriver.rs

@@ -310,6 +310,7 @@ pub fn drive_multi_frame_blackhole(
     raising_exception: bool,
     per_frame: Option<&[(i64, usize)]>,
     on_enter_level: Option<&dyn Fn(i64)>,
+    on_leave_level: Option<&dyn Fn(i64)>,
 ) -> MultiFrameBlackholeResult {
     let mut ref_locations = Vec::new();
     let mut packed_ref_roots = Vec::new();
@@ -361,6 +362,7 @@ pub fn drive_multi_frame_blackhole(
             virtualizable_stack_base,
             per_frame,
             on_enter_level,
+            on_leave_level,
         }),
         Some(&mut terminal),
     );
@@ -2308,6 +2310,7 @@ impl<S: JitState> JitDriver<S> {
             raising_exception,
             None,
             None,
+            None,
         );
         let MultiFrameBlackholeResult { outcome, terminal } = outcome;
         if crate::majit_log_enabled() {
@@ -8053,6 +8056,7 @@ impl<S: JitState> JitDriver<S> {
                         }),
                         None,
                         None,
+                        None,
                     );
                     // compile.py:716 assert 0, "unreachable"
                     if crate::majit_log_enabled() {

pyre/bench/synth/getattr_hook_binding.py

@@ -1,8 +1,15 @@
-# pyre-check: max-pypy-ratio=90
+# pyre-check: max-pypy-ratio=25
 # objspace.py:710 get_and_call_function: a __getattr__ (or __getattribute__)
 # defined as a classmethod or staticmethod must be bound through __get__ before
 # being called, exactly like any other special method, so it receives the
 # arguments the descriptor protocol gives it.
+#
+# Each of the three accesses below used to cost one opaque residual holding the
+# whole `object_getattr_miss` walk plus a fresh frame for the hook. Inlining
+# the hook against the version-tag and map pins that make the miss constant
+# dropped the ratio from 48.6x/59.5x (dynasm/wasm) to 7.4x/10.4x/8.7x
+# (dynasm/cranelift/wasm); the bound is twice the slowest of those (10.4x),
+# rounded up to the next multiple of five.
 
 
 class ClassmethodGetattr:

pyre/bench/synth/inline_freevar_after_mayforce.cranelift.jitstats

@@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0
 fbw_store_journal_rollback_failed=0
 field_pos_attached_misplaced=0
 field_pos_spec_misplaced=0
-guard_failures=1008
+guard_failures=1009
 internal_compile_panics=0
 loops_aborted=0
 loops_compiled=6

pyre/bench/synth/inlined_helper_mutation.py

@@ -1,8 +1,15 @@
-# pyre-check: max-pypy-ratio=145
+# pyre-check: max-pypy-ratio=60
 # The trip count now puts pypy above the startup-subtraction floor, so this
-# ratio is a measurement rather than pyre divided by the floor constant. The
-# ceiling is twice the slowest of the three backends observed unclamped
-# (71.1x on wasm); the previous 45 was fitted against the clamp and fails.
+# ratio is a measurement rather than pyre divided by the floor constant; a
+# ceiling fitted against the clamp (the 45 this bench once carried) fails.
+# The bound is twice the slowest of the three backends (27.7x), rounded up to
+# the next multiple of ten.
+#
+# `push` binds `a.append` inside an inlined callee, and the folds that shape a
+# bound-method load used to decline for the whole of such a sub-walk. They now
+# decline only where a guard would collapse its resume to the caller's CALL,
+# so the binding folds here: the ratio fell from 39.4x/70.2x/60.8x to
+# 15.1x/27.7x/26.4x (dynasm/cranelift/wasm).
 # Inlined-callee shared-heap mutation parity, in both helper orderings.
 #
 # A tiny helper mutates a caller-owned list/instance inside a hot while-loop,

pyre/bench/synth/property_accessor_invalidation.cranelift.jitstats

@@ -0,0 +1,15 @@
+bridges_compiled=0
+descr_set_absent=0
+descr_set_ambiguous=0
+descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=0
+fbw_blackhole_adopted_single_frame=0
+fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
+field_pos_attached_misplaced=0
+field_pos_spec_misplaced=0
+guard_failures=6
+internal_compile_panics=0
+loops_aborted=0
+loops_compiled=6
+retraces_compiled=0

pyre/bench/synth/property_accessor_invalidation.dynasm.jitstats

@@ -0,0 +1,15 @@
+bridges_compiled=0
+descr_set_absent=0
+descr_set_ambiguous=0
+descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=0
+fbw_blackhole_adopted_single_frame=0
+fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
+field_pos_attached_misplaced=0
+field_pos_spec_misplaced=0
+guard_failures=6
+internal_compile_panics=0
+loops_aborted=0
+loops_compiled=6
+retraces_compiled=0

pyre/bench/synth/property_accessor_invalidation.py

@@ -0,0 +1,93 @@
+# pyre-check: no-cpython
+# `descriptor.py:175 W_Property._immutable_fields_ = ["w_fget?", "w_fset?",
+# "w_fdel?"]`.  The `?` is what lets a tracer bake the accessor and equally what
+# registers the invalidation an assignment to the slot owes, so re-initialising
+# an installed property revokes every loop that folded it.
+#
+# CPython is not an oracle for this: its `LOAD_ATTR_PROPERTY` specialization
+# caches `fget` under the receiver type's version alone, and `property.__init__`
+# on an installed descriptor bumps no type's version, so a specialized read
+# keeps answering with the previous getter.  Cold, CPython sees the new one —
+# the divergence is the specialization's, and pyre follows pypy's `?` instead.
+#
+# Each rebind happens INSIDE its loop: a read after the loop is interpreted and
+# would not consult what the trace baked.  The accessor bodies are residual-free
+# so the folds stand rather than aborting.
+N = 400000
+SWITCH = N // 2
+
+
+def first_getter(self):
+    return 1
+
+
+def second_getter(self):
+    return 2
+
+
+def first_setter(self, value):
+    self.slot = 1
+
+
+def second_setter(self, value):
+    self.slot = 2
+
+
+class Getter:
+    x = property(first_getter)
+
+
+class Setter:
+    slot = 0
+    y = property(None, first_setter)
+
+
+def rebind_getter():
+    obj = Getter()
+    descr = Getter.__dict__['x']
+    total = 0
+    i = 0
+    while i < N:
+        total += obj.x
+        if i == SWITCH:
+            descr.__init__(second_getter)
+        i += 1
+    # SWITCH+1 reads of 1, then N-SWITCH-1 reads of 2.
+    print('getter', total)
+
+
+def rebind_setter():
+    obj = Setter()
+    descr = Setter.__dict__['y']
+    total = 0
+    i = 0
+    while i < N:
+        obj.y = i
+        total += obj.slot
+        if i == SWITCH:
+            descr.__init__(None, second_setter)
+        i += 1
+    print('setter', total)
+
+
+def drop_getter():
+    # The sharper case: the re-init leaves no getter at all, and `W_Property.get`
+    # (descriptor.py:224-225) raises rather than calling the old function.
+    obj = Getter()
+    descr = Getter.__dict__['x']
+    raised = 0
+    i = 0
+    while i < N:
+        try:
+            obj.x
+        except AttributeError:
+            raised += 1
+        if i == SWITCH:
+            descr.__init__(None)
+        i += 1
+    print('dropped', raised)
+
+
+rebind_getter()
+rebind_setter()
+drop_getter()

pyre/bench/synth/property_accessor_invalidation.wasm.jitstats

@@ -0,0 +1,15 @@
+bridges_compiled=0
+descr_set_absent=0
+descr_set_ambiguous=0
+descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=0
+fbw_blackhole_adopted_single_frame=0
+fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
+field_pos_attached_misplaced=0
+field_pos_spec_misplaced=0
+guard_failures=6
+internal_compile_panics=0
+loops_aborted=0
+loops_compiled=6
+retraces_compiled=0

pyre/bench/synth/range_ctor_in_loop.cranelift.jitstats

@@ -1,4 +1,4 @@
-bridges_compiled=0
+bridges_compiled=3
 descr_set_absent=0
 descr_set_ambiguous=0
 descr_set_stale_absent=0
@@ -8,7 +8,8 @@ fbw_rolled_back_with_effects=0
 fbw_store_journal_rollback_failed=0
 field_pos_attached_misplaced=0
 field_pos_spec_misplaced=0
-guard_failures=0
+guard_failures=812
 internal_compile_panics=0
 loops_aborted=0
-loops_compiled=1
+loops_compiled=3
+retraces_compiled=0

pyre/bench/synth/range_ctor_in_loop.dynasm.jitstats

@@ -1,4 +1,4 @@
-bridges_compiled=0
+bridges_compiled=3
 descr_set_absent=0
 descr_set_ambiguous=0
 descr_set_stale_absent=0
@@ -8,7 +8,8 @@ fbw_rolled_back_with_effects=0
 fbw_store_journal_rollback_failed=0
 field_pos_attached_misplaced=0
 field_pos_spec_misplaced=0
-guard_failures=0
+guard_failures=812
 internal_compile_panics=0
 loops_aborted=0
-loops_compiled=1
+loops_compiled=3
+retraces_compiled=0

pyre/bench/synth/range_ctor_in_loop.py

@@ -1,4 +1,4 @@
-# pyre-check: max-pypy-ratio=190
+# pyre-check: max-pypy-ratio=96
 # Pins virtual range construction for one-, two-, and three-bound calls while
 # retaining correct residual behavior for exceptional, subclass, index, and
 # escaping-object shapes.
@@ -9,11 +9,17 @@
 # pypy spends 0.10s, clearing the floor even on the platform with the
 # coarsest timer.
 #
-# The ceiling rose from 50 because that bound was fitted to the floored
-# denominator, not because anything got slower: the honest ratio here is
-# 83x as a median of interleaved pairwise runs. It is dominated by the four
-# deliberately residual shapes below rather than by the virtualized loops --
-# each iteration also raises and catches a ValueError.
+# `main` used to run interpreted end to end. The `try: range(0, 3, 0)` below
+# puts an out-of-line handler after the trailing comprehension, and the loop
+# region that gates the back edge grew across the gap between them and picked
+# up that comprehension's call-bearing `FOR_ITER` -- an opcode this loop never
+# reaches. With the region built from the exception table instead, the while
+# loop and the three `for` loops compile, and this gate's own metric falls
+# from 122x to 23.2x dynasm / 30.2x cranelift / 24.7x wasm. A separate
+# min-of-five interleaved harness reads the same move as 158x to 35.2x /
+# 38.3x, so the ceiling is set from the slower of the two readings: 2.5x of
+# 38.3x, the slack every bench here carries against a runner 2.5x slower than
+# an idle local box.
 N = 400000