jit-trace: exactness gates on the int/float payload folds; dynasm cond-call predicate from arglocs

majit/majit-backend-dynasm/src/aarch64/assembler.rs

@@ -3122,7 +3122,7 @@ impl<'a> AssemblerARM64<'a> {
             }
             OpCode::CondCallN => self.genop_discard_cond_call(op, arglocs),
             OpCode::CondCallValueI | OpCode::CondCallValueR => {
-                self.genop_cond_call_value(op);
+                self.genop_cond_call_value(op, arglocs);
             }
             // ── Allocation (raw, when GC rewriter is not active) ──
             OpCode::New => self.genop_new(op),
@@ -6651,8 +6651,14 @@ impl<'a> AssemblerARM64<'a> {
     }
 
     /// COND_CALL_VALUE_I/R: if arg(0) == 0, call function; else result = arg(0).
-    fn genop_cond_call_value(&mut self, op: &Op) {
-        self.load_arg_to_rax(op.arg(0).to_opref());
+    ///
+    /// The predicate comes from its regalloc location, not `resolve_opref`,
+    /// which only recognises constants and frame slots — a predicate left
+    /// register-resident has no slot mapping.  It is loaded into x0 rather
+    /// than the ip0 scratch because on the not-taken path the predicate IS the
+    /// result, and `store_rax_to_result` reads it from there.
+    fn genop_cond_call_value(&mut self, op: &Op, arglocs: &[Loc]) {
+        self.emit_load_to_rax(arglocs[0]);
         let skip_label = self.mc.new_dynamic_label();
         dynasm!(self.mc ; .arch aarch64 ; cbnz x0, =>skip_label);
 

majit/majit-backend-dynasm/src/x86/assembler.rs

@@ -1427,6 +1427,19 @@ impl<'a> Assembler386<'a> {
         }
     }
 
+    /// Emit: load a regalloc Loc into the dedicated scratch (R11), which is
+    /// outside `ALL_CORE_REGS`, so the load cannot clobber a value the
+    /// regalloc still has live in an allocatable register.  Counterpart of
+    /// the AArch64 `emit_load_loc_to_ip0`.
+    fn emit_load_loc_to_scratch(&mut self, loc: Loc) {
+        let scratch = crate::regloc::X86_64_SCRATCH_REG;
+        match loc {
+            // already there
+            Loc::Reg(r) if !r.is_xmm && r.value == scratch.value => {}
+            _ => self.regalloc_mov(&loc, &Loc::Reg(scratch)),
+        }
+    }
+
     /// Emit: load the value of `opref` into RCX (x64) / X1 (aarch64).
     fn load_arg_to_rcx(&mut self, opref: OpRef) {
         match self.resolve_opref(opref) {
@@ -4234,7 +4247,7 @@ impl<'a> Assembler386<'a> {
             }
             OpCode::CondCallN => self.genop_discard_cond_call(op, arglocs),
             OpCode::CondCallValueI | OpCode::CondCallValueR => {
-                self.genop_cond_call_value(op);
+                self.genop_cond_call_value(op, arglocs);
             }
             // ── Allocation (raw, when GC rewriter is not active) ──
             OpCode::New => self.genop_new(op),
@@ -7969,8 +7982,18 @@ impl<'a> Assembler386<'a> {
         if let Some(cc) = self.guard_success_cc.take() {
             self.emit_jcc_to_label(invert_cc(cc), skip_label);
         } else {
-            self.load_arg_to_rax(op.arg(0).to_opref());
-            dynasm!(self.mc ; .arch x64 ; test rax, rax ; jz =>skip_label);
+            // Read the predicate from its regalloc location, not via
+            // `resolve_opref`: `consider_discard_nargs_j2` emits no
+            // `before_call`, so a predicate the regalloc left register-resident
+            // has no slot mapping and would panic there.  Test it in the
+            // scratch (R11) rather than rax, which IS allocatable here and may
+            // still hold one of the call's own arglocs.
+            self.emit_load_loc_to_scratch(arglocs[0]);
+            let scratch = crate::regloc::X86_64_SCRATCH_REG.value;
+            dynasm!(self.mc ; .arch x64
+                ; test Rq(scratch), Rq(scratch)
+                ; jz =>skip_label
+            );
         }
 
         // `consider_discard_nargs` emits no `before_call`, so the regalloc
@@ -7996,8 +8019,15 @@ impl<'a> Assembler386<'a> {
     }
 
     /// COND_CALL_VALUE_I/R: if arg(0) == 0, call function; else result = arg(0).
-    fn genop_cond_call_value(&mut self, op: &Op) {
-        self.load_arg_to_rax(op.arg(0).to_opref());
+    ///
+    /// The predicate comes from its regalloc location for the same reason as
+    /// `genop_discard_cond_call`.  It is loaded into rax rather than the
+    /// scratch because on the not-taken path the predicate IS the result, and
+    /// `store_rax_to_result` reads it from there.  `consider_raw_call_like_j2`
+    /// runs `before_call` before computing arglocs, so no argloc is a
+    /// caller-saved register and this load cannot clobber one.
+    fn genop_cond_call_value(&mut self, op: &Op, arglocs: &[Loc]) {
+        self.emit_load_to_rax(arglocs[0]);
         let skip_label = self.mc.new_dynamic_label();
         dynasm!(self.mc ; .arch x64 ; test rax, rax ; jnz =>skip_label);
 

pyre/bench/synth/float_subclass_binop_dispatch.cranelift.jitstats

@@ -8,8 +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=17
+guard_failures=24
 internal_compile_panics=0
 loops_aborted=0
-loops_compiled=17
+loops_compiled=24
 retraces_compiled=0

pyre/bench/synth/float_subclass_binop_dispatch.dynasm.jitstats

@@ -8,8 +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=17
+guard_failures=24
 internal_compile_panics=0
 loops_aborted=0
-loops_compiled=17
+loops_compiled=24
 retraces_compiled=0

pyre/bench/synth/float_subclass_binop_dispatch.py

@@ -196,6 +196,56 @@ def warm_then_swap_store_attr(n):
     return type(holder.x).__name__
 
 
+# The same three storage paths on the float side. The value's exact class must
+# survive the store: a `LiarFloat` written through a warmed-up float store must
+# read back as `LiarFloat`, not as `float`.
+def warm_then_swap_store_subscr_float(n):
+    lst = [0.0]
+    for a in [0.0] * n + [LiarFloat(7.0)]:
+        lst[0] = a
+    return type(lst[0]).__name__
+
+
+def warm_then_swap_newlist_float(n):
+    out = None
+    for a in [0.0] * n + [LiarFloat(7.0)]:
+        out = [a]
+    return type(out[0]).__name__
+
+
+def warm_then_swap_store_attr_float(n):
+    holder = Slotted()
+    for a in [0.0] * n + [LiarFloat(7.0)]:
+        holder.x = a
+    return type(holder.x).__name__
+
+
+# Cold start on the storage side: the subclass is the value from the first
+# iteration, so the unboxed-strategy gates must decline on the recorded value
+# rather than rely on the `w_class` pin to reject a later arrival. An unboxed
+# int/float slot stores the raw payload, so a fold here reads back as `int` /
+# `float` instead of the subclass.
+def store_subscr_cold_subclass(n):
+    lst = [0]
+    for _ in range(n):
+        lst[0] = LiarInt(7)
+    return type(lst[0]).__name__
+
+
+def store_attr_cold_subclass(n):
+    holder = Slotted()
+    for _ in range(n):
+        holder.x = LiarInt(7)
+    return type(holder.x).__name__
+
+
+def store_attr_cold_subclass_float(n):
+    holder = Slotted()
+    for _ in range(n):
+        holder.x = LiarFloat(7.0)
+    return type(holder.x).__name__
+
+
 # `truth_int` reaches the same hole from the branch side rather than the value
 # side: `POP_JUMP_IF_*` and the short-circuit operators read the truth of a
 # payload the `GUARD_CLASS INT` admits, so a `__bool__` override on a zero-payload
@@ -216,6 +266,20 @@ def warm_then_swap_truth_and(n):
     return out
 
 
+# Cold start: the trace records on the subclass from the very first iteration
+# rather than meeting it after warming up on exact ints. The fold has to decline
+# on the *recorded* operand — pinning `w_class` only rejects a subclass that
+# arrives later, and the walk is the authoritative executor, so a payload-folded
+# truth here is the answer the program returns. `LiarBool(0)` is falsy by payload
+# and true by `__bool__`, so the two answers differ.
+def truth_cold_subclass(n):
+    hits = 0
+    for a in [LiarBool(0)] * n:
+        if a:
+            hits += 1
+    return hits
+
+
 def truth_bool_call_control(n):
     out = None
     for a in [0] * n + [LiarBool(0)]:
@@ -228,6 +292,13 @@ def truth_bool_call_control(n):
 print(warm_then_swap_store_subscr(N))
 print(warm_then_swap_newlist(N))
 print(warm_then_swap_store_attr(N))
+print(warm_then_swap_store_subscr_float(N))
+print(warm_then_swap_newlist_float(N))
+print(warm_then_swap_store_attr_float(N))
 print(warm_then_swap_truth_if(N))
 print(warm_then_swap_truth_and(N))
+print(store_subscr_cold_subclass(N))
+print(store_attr_cold_subclass(N))
+print(store_attr_cold_subclass_float(N))
+print(truth_cold_subclass(N))
 print(truth_bool_call_control(N))

pyre/bench/synth/float_subclass_binop_dispatch.wasm.jitstats

@@ -8,8 +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=17
+guard_failures=24
 internal_compile_panics=0
 loops_aborted=0
-loops_compiled=17
+loops_compiled=24
 retraces_compiled=0

pyre/pyre-interpreter/src/module/math/interp_math.rs

@@ -30,24 +30,10 @@ pub fn try_get_double(obj: PyObjectRef) -> Result<f64, crate::PyError> {
         // subclass falls to the ladder below; an inherited `int.__float__`
         // reproduces the payload.  The `float` arm stays ungated because the
         // conversion short-circuits on the layout and ignores an override.
-        if pyre_object::is_exact_builtin_instance(obj) {
-            if is_int(obj) {
-                return Ok(w_int_get_value(obj) as f64);
-            }
-            if is_long(obj) {
-                // A Python int is always finite, so a non-finite conversion means
-                // the magnitude exceeds f64 range — PyFloat_AsDouble raises here.
-                let v = jit_bigint_to_f64_or_nan(w_long_get_value(obj));
-                if !v.is_finite() {
-                    return Err(crate::PyError::overflow_error(
-                        "int too large to convert to float",
-                    ));
-                }
-                return Ok(v);
-            }
-            if is_bool(obj) {
-                return Ok(if w_bool_get_value(obj) { 1.0 } else { 0.0 });
-            }
+        if pyre_object::is_exact_builtin_instance(obj)
+            && let Some(value) = crate::builtins::int_payload_as_f64(obj)
+        {
+            return value;
         }
     }
     // `__float__` is a type-only special-method lookup (`space.lookup`); an

pyre/pyre-interpreter/src/objspace/std/mapdict.rs

@@ -3404,7 +3404,7 @@ fn box_value(typ: UnboxType, val: i64) -> PyObjectRef {
 ///
 /// # Safety
 /// `w_value` must point to a live object.
-unsafe fn is_unboxable_int(w_value: PyObjectRef) -> bool {
+pub unsafe fn is_unboxable_int(w_value: PyObjectRef) -> bool {
     if unsafe { pyre_object::is_bool(w_value) } || !unsafe { pyre_object::is_int(w_value) } {
         return false;
     }

pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs

@@ -181,7 +181,19 @@ pub(crate) fn try_walker_specialize_truth_int<Sym: WalkSym>(
         return Ok(None);
     };
     let val = unsafe {
-        if !pyre_object::is_int(obj) || pyre_object::is_bool(obj) {
+        // `is_int` reads `ob_type`, which an `int` subclass shares, so it alone
+        // admits one here.  Two things then go wrong at once: the walk folds the
+        // truth straight off the payload instead of running the subclass's
+        // `__bool__`, and `walker_numeric_builtin_class` answers with the
+        // canonical `int` — a `w_class` the recorded operand does not carry, so
+        // the pin below becomes a guard that fails on the very value that
+        // recorded it.  Decline before unboxing, as `walker_unary_int_operand`
+        // does; `walker_numeric_builtin_class` documents this gate as its
+        // precondition.
+        if !pyre_object::is_int(obj)
+            || pyre_object::is_bool(obj)
+            || !pyre_object::is_exact_builtin_instance(obj)
+        {
             return Ok(None);
         }
         pyre_object::w_int_get_value(obj)
@@ -3898,11 +3910,12 @@ pub(crate) fn try_walker_specialize_store_attr<Sym: WalkSym>(
     } {
         match unbox_type {
             pyre_interpreter::objspace::std::mapdict::UnboxType::Int => {
-                // `type(w_value) is space.IntObjectCls` (mapdict.py): reject bool
-                // and every type-changing value before emitting any guards.
-                if unsafe {
-                    pyre_object::pyobject::is_bool(concrete_value)
-                        || !pyre_object::pyobject::is_int(concrete_value)
+                // Match mapdict.py `_direct_write` exactly, through the same
+                // predicate the interpreter's own store uses: `is_int` reads
+                // `ob_type`, which an `int` subclass shares, so unboxing on it
+                // would take the raw payload and lose `w_class`.
+                if !unsafe {
+                    pyre_interpreter::objspace::std::mapdict::is_unboxable_int(concrete_value)
                 } {
                     return Ok(None);
                 }
@@ -14234,8 +14247,16 @@ pub(crate) fn try_walker_specialize_store_subscr<Sym: WalkSym>(
         if index as usize >= concrete_len {
             return Ok(None);
         }
+        // Object storage keeps the value boxed, so a subclass survives it; the
+        // unboxed strategies write the raw payload and would drop the subclass
+        // identity the read-back must return.  `is_int`/`is_float` read
+        // `ob_type`, which a subclass shares, so they alone do not establish
+        // that -- and `walker_numeric_builtin_class` below answers with the
+        // canonical class, which such a value does not carry.
         let sid = if pyre_object::w_list_uses_object_storage(list_obj) {
             0i64
+        } else if !pyre_object::is_exact_builtin_instance(value_obj) {
+            return Ok(None);
         } else if pyre_object::w_list_uses_int_storage(list_obj)
             && pyre_object::is_int(value_obj)
             && !pyre_object::is_bool(value_obj)