diff --git a/pyre/bench/synth/comprehension_object_append_hot.py b/pyre/bench/synth/comprehension_object_append_hot.py index 080ec9ffbaf..40e941cc3eb 100644 --- a/pyre/bench/synth/comprehension_object_append_hot.py +++ b/pyre/bench/synth/comprehension_object_append_hot.py @@ -1,3 +1,4 @@ +# pyre-check: max-pypy-ratio=40 # An inlined list comprehension whose LIST_APPEND element lands in a list # Object-strategy (tuple / None / str / dict / f-string) folds through the #171 # orthodox append. Its Object arm stores a GC ref and runs list_write_barrier, diff --git a/pyre/bench/synth/inlined_helper_arith_hot.py b/pyre/bench/synth/inlined_helper_arith_hot.py new file mode 100644 index 00000000000..400756f2cda --- /dev/null +++ b/pyre/bench/synth/inlined_helper_arith_hot.py @@ -0,0 +1,190 @@ +# pyre-check: max-pypy-ratio=12 +# One-line arithmetic helpers called from a hot `for` loop body. The inline +# lever gates a FOR_ITER-in-flight callee on `fbw_callee_body_replay_safety`: +# a body whose only residual is a BINARY_OP the walker will specialize to a +# native op leaves nothing to replay and is admitted, anything else is Dirty +# and the whole callee stays a per-iteration residual call. +# +# The accepted tag set is every tag a specialization table lowers with no +# runtime decline left. Add / Subtract / Multiply are in both the int +# (IntAddOvf / IntSubOvf / IntMulOvf) and float (FloatAdd / FloatSub / +# FloatMul) tables; And / Or / Xor are in the int table only. Both tables key +# each in-place tag to the same arm as its plain form, so `a += i` is admitted +# exactly like `a + i`. Each loop below pins one tag, so a tag dropping out of +# the set shows up here as the callee reappearing as a residual and the ratio +# blowing past the gate. +# +# The divide / remainder / shift / power tags are deliberately NOT in the set +# — each can still decline (zero divisor, out-of-range shift, nan/inf base) and +# leave the residual behind — and they are kept out of this file so the gate +# measures the admitted paths rather than cases that are meant to stay slow. +# Callees are named directly rather than passed in, so the call site keeps a +# constant callee. Output verified against CPython/PyPy. +N = 200000 + + +def add_body(a, i): + return a + i + + +def sub_body(a, i): + return a - i + + +def mul_body(a, i): + return a + i * 2 + + +def iadd_body(a, i): + a += i + return a + + +def isub_body(a, i): + a -= i + return a + + +def imul_body(a, i): + b = i + b *= 2 + return a + b + + +def and_body(a, i): + return a + (i & 255) + + +def or_body(a, i): + return a + (i | 1) + + +def xor_body(a, i): + return a + (i ^ 5) + + +def ibit_body(a, i): + b = i + b &= 255 + b |= 1 + b ^= 5 + return a + b + + +def mixed_body(a, i): + return a + i * 3 - 1 + + +def float_body(a, i): + return a + i * 0.5 + + +def float_iadd_body(a, i): + a += i * 0.25 + return a + + +def run_add(n): + s = 0 + for i in range(n): + s = add_body(s, i) + return s + + +def run_sub(n): + s = 0 + for i in range(n): + s = sub_body(s, i) + return s + + +def run_mul(n): + s = 0 + for i in range(n): + s = mul_body(s, i) + return s + + +def run_iadd(n): + s = 0 + for i in range(n): + s = iadd_body(s, i) + return s + + +def run_isub(n): + s = 0 + for i in range(n): + s = isub_body(s, i) + return s + + +def run_imul(n): + s = 0 + for i in range(n): + s = imul_body(s, i) + return s + + +def run_and(n): + s = 0 + for i in range(n): + s = and_body(s, i) + return s + + +def run_or(n): + s = 0 + for i in range(n): + s = or_body(s, i) + return s + + +def run_xor(n): + s = 0 + for i in range(n): + s = xor_body(s, i) + return s + + +def run_ibit(n): + s = 0 + for i in range(n): + s = ibit_body(s, i) + return s + + +def run_mixed(n): + s = 0 + for i in range(n): + s = mixed_body(s, i) + return s + + +def run_float(n): + s = 0.0 + for i in range(n): + s = float_body(s, i) + return s + + +def run_float_iadd(n): + s = 0.0 + for i in range(n): + s = float_iadd_body(s, i) + return s + + +print(run_add(N)) +print(run_sub(N)) +print(run_mul(N)) +print(run_iadd(N)) +print(run_isub(N)) +print(run_imul(N)) +print(run_and(N)) +print(run_or(N)) +print(run_xor(N)) +print(run_ibit(N)) +print(run_mixed(N)) +print(run_float(N)) +print(run_float_iadd(N)) diff --git a/pyre/bench/synth/list_append_write_barrier_gc.py b/pyre/bench/synth/list_append_write_barrier_gc.py new file mode 100644 index 00000000000..925a80fd321 --- /dev/null +++ b/pyre/bench/synth/list_append_write_barrier_gc.py @@ -0,0 +1,92 @@ +# GC stress for the in-place Object-append write barrier the tracer does NOT +# record. `w_list_append`'s in-place arm stores a GC ref into the items block +# and runs `list_write_barrier`; when the block is GC-managed the backend GC +# rewrite already marks that store with COND_CALL_GC_WB_ARRAY, so the walker +# drops the barrier residual instead of leaving a second barrier in the loop +# (rewrite.py:936-944; pyjitpl records no write barrier at all, +# executor.py:446). These cases pin what that suppression must not break: an +# OLD list whose block is already promoted receiving YOUNG elements. If the +# store went unremembered, a minor collection would never scan the slot and the +# element would be freed or read back stale. Run with PYRE_GC_ITEMSBLOCK=0 too +# — there the block is std::alloc with no GC header, the barrier on the +# W_ListObject is load-bearing, and the walker must keep emitting it. +# Output verified against CPython/PyPy. +N = 4000 +CHURN = 300 + + +def churn(k): + # Allocation pressure to drive minor collections between appends. + junk = None + for i in range(k): + junk = (i, [i], {i: i}) + return junk + + +def old_list_young_appends(): + r = [] + for i in range(N): + r.append((i, i)) + churn(CHURN * 20) + # r and its block are old now; append fresh young tuples. + for i in range(N): + r.append((i + N, i + N)) + if i % 200 == 0: + churn(CHURN) + assert len(r) == 2 * N, len(r) + total = 0 + for a, b in r: + assert a == b, (a, b) + total += a + return total + + +def interleaved_growth(): + lists = [[] for _ in range(16)] + total = 0 + for i in range(N): + lists[i % 16].append((i,)) + if i % 100 == 0: + churn(CHURN) + for lst in lists: + for (v,) in lst: + total += v + return total, sum(len(x) for x in lists) + + +def strings_and_dicts(): + # Non-tuple Object-strategy elements: str and dict payloads that the + # collector must keep reachable through the appended slots. + r = [] + for i in range(N): + r.append(str(i)) + if i % 250 == 0: + churn(CHURN) + joined = 0 + for i, s in enumerate(r): + assert s == str(i), (i, s) + joined += len(s) + return joined + + +def none_then_objects(): + r = [None] * 8 + r.clear() + for i in range(N): + r.append(None if i % 2 else [i]) + if i % 300 == 0: + churn(CHURN) + total = 0 + for i, v in enumerate(r): + if i % 2: + assert v is None, (i, v) + else: + assert v == [i], (i, v) + total += v[0] + return total + + +print(old_list_young_appends()) +print(interleaved_growth()) +print(strings_and_dicts()) +print(none_then_objects()) diff --git a/pyre/pyre-interpreter/src/jit_fnaddr.rs b/pyre/pyre-interpreter/src/jit_fnaddr.rs index 992769a574b..8134aaae1a0 100644 --- a/pyre/pyre-interpreter/src/jit_fnaddr.rs +++ b/pyre/pyre-interpreter/src/jit_fnaddr.rs @@ -1219,10 +1219,14 @@ pub fn jit_trace_fnaddrs() -> Vec<(&'static str, i64)> { ); // The #171 object-append fold descends `w_list_append` and folds the // store leaves to native ops, leaving `list_write_barrier(l)` as a - // residual call (the off-GC ItemsBlock is reached by the collector only - // through the remembered W_ListObject). Register it so the codewriter - // resolves the residual to a runtime-patchable address instead of a - // `symbolic_fnaddr_for_path` hash the inline sub-walk must decline. + // residual call. Register it so the codewriter resolves the residual to a + // runtime-patchable address instead of a `symbolic_fnaddr_for_path` hash + // the inline sub-walk must decline. The address is also what the walker + // matches on to drop the residual entirely when the backend GC rewrite + // already covers the store (`FbwWalkMode::append_inplace_wb_covered`); + // with an off-GC ItemsBlock the residual stays, because there the + // collector reaches the block's slots only through the remembered + // `W_ListObject`. push_alias_pair( &mut entries, "pyre_object::listobject::list_write_barrier", diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs index 128a1e21448..251f36d23c9 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs @@ -1411,7 +1411,8 @@ pub(crate) enum CalleeReplaySafety { /// there, which this straight-line scan cannot name. pub(crate) fn fbw_callee_body_replay_safety( body_code: &[u8], - args_all_numeric: bool, + args_all_exact_numeric: bool, + args_all_exact_plain_int: bool, num_regs_i: usize, constants_i: &[i64], callee_descr_refs: &[DescrRef], @@ -1465,9 +1466,10 @@ pub(crate) fn fbw_callee_body_replay_safety( || ei.check_is_elidable() || ei.extraeffect == majit_ir::ExtraEffect::LoopInvariant; if !provably_side_effect_free - && !residual_call_is_specialized_plain_int_add( + && !residual_call_is_specialized_plain_numeric_binop( body_code, - args_all_numeric, + args_all_exact_numeric, + args_all_exact_plain_int, &d, num_regs_i, constants_i, diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index 9e2afd464f7..0ba56e85601 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -1417,6 +1417,15 @@ pub(crate) fn try_walker_inline_user_call( if pyre_helper != majit_ir::PyreHelperKind::CallFn && !is_call_kw && !is_call_function_ex { return Ok(None); } + if std::env::var_os("PYRE_FBW_INLINE_DIAG").is_some() { + eprintln!( + "[inline-entry] pc={} helper={:?} nrefargs={} subwalk={}", + op.pc, + pyre_helper, + r_args.len(), + ctx.fbw_mode.inline_subwalk, + ); + } if r_args.is_empty() { return Ok(None); } @@ -1450,8 +1459,17 @@ pub(crate) fn try_walker_inline_user_call( let method_form = !null_or_self.is_null() && null_or_self != pyre_object::PY_NULL; let Some((w_code, nparams, has_closure)) = (unsafe { resolve_inlinable_callee(callable) }) else { + if std::env::var_os("PYRE_FBW_INLINE_DIAG").is_some() { + eprintln!("[inline-decline] pc={} callee not inlinable", op.pc); + } return Ok(None); }; + if std::env::var_os("PYRE_FBW_INLINE_DIAG").is_some() { + eprintln!( + "[inline-resolved] pc={} nparams={nparams} has_closure={has_closure} method_form={method_form}", + op.pc, + ); + } let (callee_args, callee_arg_concretes) = if is_call_kw { // A keyword call folds its `kwnames`->parameter permutation at trace // time (`fbw_reorder_call_kw_args`) so the reordered param-order args @@ -1574,13 +1592,46 @@ pub(crate) fn try_walker_inline_resolved_user_call( else { return Ok(None); }; - let args_all_numeric = callee_arg_concretes.iter().all(|concrete| match concrete { - ConcreteValue::Int(_) | ConcreteValue::Float(_) | ConcreteValue::Bool(_) => true, - ConcreteValue::Ref(obj) if !obj.is_null() => unsafe { - pyre_object::is_int(*obj) || pyre_object::is_float(*obj) - }, - ConcreteValue::Ref(_) | ConcreteValue::Null => false, - }); + // EXACT int/float only. These feed `fbw_callee_body_replay_safety`, whose + // question is "will the walker specialize this body's BINARY_OP to a native + // op, leaving no residual to replay?". The walker's specialization admits + // only exact builtin numbers (`walker_int_specialization_operands` / + // `walker_float_specialization_operands` both require + // `is_exact_builtin_instance`), because a numeric subclass keeps the + // builtin layout while its Python-visible class lives in `w_class` and may + // define its own `__add__`. `is_int` / `is_float` are `ob_type` checks + // that a subclass passes, so using them here claims a specialization that + // will not happen and admits a body whose real residual a replay would + // double. + // + // The two widths are folded in one pass because the accepted tag set + // depends on which one holds: only the int table covers `And` / `Or` / + // `Xor`, so those need every argument to be an exact plain int, while + // `Add` / `Subtract` / `Multiply` are in both tables and take either. + // `bool` is excluded from both: `is_plain_int1` rejects it, so an + // argument list carrying one stays on the conservative side. + let (args_all_exact_plain_int, args_all_exact_numeric) = + callee_arg_concretes + .iter() + .fold((true, true), |(all_int, all_numeric), concrete| { + let (exact_int, exact_float) = match concrete { + ConcreteValue::Int(_) => (true, false), + ConcreteValue::Float(_) => (false, true), + ConcreteValue::Ref(obj) if !obj.is_null() => unsafe { + ( + pyre_object::is_plain_int1(*obj), + pyre_object::is_plain_float_strict(*obj), + ) + }, + ConcreteValue::Bool(_) | ConcreteValue::Ref(_) | ConcreteValue::Null => { + (false, false) + } + }; + ( + all_int && exact_int, + all_numeric && (exact_int || exact_float), + ) + }); let args_all_builtin_integer = callee_arg_concretes.iter().all(|concrete| match concrete { ConcreteValue::Int(_) | ConcreteValue::Bool(_) => true, ConcreteValue::Ref(obj) if !obj.is_null() => unsafe { pyre_object::is_int_or_long(*obj) }, @@ -1658,13 +1709,15 @@ pub(crate) fn try_walker_inline_resolved_user_call( // per iteration, though each body on its own is pure arithmetic. let mut foriter_deferred_admit = false; if fbw_foriter_inflight_active() { - let admit = match fbw_callee_body_replay_safety( + let safety = fbw_callee_body_replay_safety( body.code, - args_all_numeric, + args_all_exact_numeric, + args_all_exact_plain_int, body.num_regs_i, body.constants_i, callee_descr_refs, - ) { + ); + let admit = match safety { CalleeReplaySafety::Clean => true, CalleeReplaySafety::DeferredCall => { foriter_deferred_admit = !fbw_foriter_deferred_call_denied(callee_code_key); @@ -1672,6 +1725,13 @@ pub(crate) fn try_walker_inline_resolved_user_call( } CalleeReplaySafety::Dirty => false, }; + if std::env::var_os("PYRE_FBW_INLINE_DIAG").is_some() { + eprintln!( + "[inline-foriter-gate] pc={} admit={admit} exact_numeric={args_all_exact_numeric} \ + safety={safety:?} deferred_admit={foriter_deferred_admit}", + op.pc, + ); + } if !admit { return Ok(None); } diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs index 7b76023f77a..9bf92c9d297 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs @@ -741,6 +741,29 @@ pub struct FbwWalkMode { /// boundary rather than mapping the callee `op_pc` through the outer /// jitcode in `walker_capture_snapshot_for_last_guard`. pub inline_subwalk: bool, + /// The enclosing `w_list_append` fold took the Object strategy's in-place + /// arm, so the appended ref lands in a `SetarrayitemGc` the backend GC + /// rewrite already covers with `COND_CALL_GC_WB_ARRAY` + /// (`rewrite.py:936-944` `handle_write_barrier_setarrayitem`). The list's + /// own `items` pointer is unchanged on that arm, so remembering the + /// `W_ListObject` adds nothing the array barrier does not already do. + /// + /// `list_write_barrier` still RUNS concretely during the walk — the walk + /// mutates the live heap — but recording it would leave a barrier call in + /// the compiled trace, and upstream emits none: pyjitpl never executes a + /// write barrier (`executor.py:446`), `COND_CALL_GC_WB` is neither + /// can-raise nor a call (`resoperation.py:1124-1125`), and the only + /// barrier in a compiled loop is the one the backend rewrite inserts. + /// + /// Carries the receiver address rather than a flag so only that list's + /// barrier is dropped — a barrier reached for any other list inside the + /// sub-walk is recorded normally. + /// + /// Only set while the items block is GC-managed. With + /// `PYRE_GC_ITEMSBLOCK=0` the block is `std::alloc` memory with no GC + /// header, the collector reaches its slots only through the remembered + /// `W_ListObject`, and the hand barrier is load-bearing. + pub append_inplace_wb_covered_receiver: Option, /// A bridge-carrier resume folds nested self-recursive calls directly to /// `CALL_ASSEMBLER` (`opimpl_recursive_call_assembler`) rather than /// re-unrolling the call tree to the multi-frame depth cap. @@ -806,6 +829,7 @@ impl Default for FbwWalkMode { Self { snapshot_sym: std::ptr::null(), inline_subwalk: false, + append_inplace_wb_covered_receiver: None, carrier_resume: false, current_exception_seed: None, current_exception_seed_concrete: pyre_object::PY_NULL, diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs index a1d0dde233a..c648c89a6c4 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs @@ -1153,8 +1153,8 @@ pub(crate) fn try_execute_residual_call_via_executor( // arm leaves it as a residual because it is `#[dont_look_inside]`) is pure // idempotent GC bookkeeping — re-running it on a body replay only re-adds // the list to the remembered set, never doubling user-visible state. It - // must still EXECUTE concretely below (pyre has no backend GC-rewrite pass, - // so the barrier runs during the walk for GC correctness), but it is not a + // must still EXECUTE concretely below — the walk mutates the live heap, so + // the store it guards has really happened — but it is not a // body effect: keep it out of the in-flight-FOR_ITER body-effect accounting // so an Object-strategy comprehension append (`[(i, i) for …]`, `[None …]`) // is not refuse-dropped. RPython treats the write barrier the same way — @@ -1162,7 +1162,9 @@ pub(crate) fn try_execute_residual_call_via_executor( // (`rpython/jit/metainterp/executor.py:446`), is neither can-raise nor a // call (`resoperation.py:1124-1125`), and is inserted only by the backend // GC rewrite pass after optimization (`backend/llsupport/rewrite.py:948`), - // so it never participates in the metainterp's side-effect analysis. + // so it never participates in the metainterp's side-effect analysis. On + // the in-place Object-append arm it is not recorded at all, for the same + // reason — see `FbwWalkMode::append_inplace_wb_covered`. let is_idempotent_gc_barrier = pyre_interpreter::is_list_write_barrier(func_ptr as usize); if allboxes.len() - 1 > majit_translate::codewriter::insns::MAX_HOST_CALL_ARITY { return Ok(ResidualExecOutcome::Declined(ResidualDecline::Symbolic)); @@ -2217,23 +2219,64 @@ pub(crate) fn residual_call_descr_index_in_body(body_code: &[u8], d: &DecodedOp) Some(decode_descr_index(body_code, d, descr_offset)) } -/// `BINARY_OP Add` has the generic residual shape in a per-function jitcode, -/// but the walker replaces a statically tagged plain add with `IntAddOvf` or -/// `FloatAdd` before the generic residual executor (and its nested-residual -/// decline) is reached when every incoming callee argument is int or float. +/// A `BINARY_OP` has the generic residual shape in a per-function jitcode, but +/// the walker replaces a statically tagged plain arithmetic op with a native +/// one before the generic residual executor (and its nested-residual decline) +/// is reached, when every incoming callee argument is an exact int or float. /// Non-numeric operands stay an impure residual, so admitting them here would -/// trigger the nested-residual 6421 abort storm. Accept only the constant -/// `Add` tag with numeric arguments; every in-place tag and every dynamic or -/// different binary operation remains conservative. -pub(crate) fn residual_call_is_specialized_plain_int_add( +/// trigger the nested-residual 6421 abort storm. +/// +/// The accepted set is every tag a specialization table lowers with no runtime +/// decline path left, so nothing survives as a residual. Both tables key the +/// in-place tag to the SAME arm as its plain form, so the two forms are +/// admitted together: +/// +/// - `Add` / `Subtract` / `Multiply` (+ in-place) — `IntAddOvf` / `IntSubOvf` / +/// `IntMulOvf` in `try_walker_specialize_binary_op_int`, `FloatAdd` / +/// `FloatSub` / `FloatMul` in `try_walker_specialize_binary_op_float`. In +/// both tables `needs_concrete_check` is false, so either argument width +/// lowers unconditionally. +/// - `And` / `Or` / `Xor` (+ in-place) — `IntAnd` / `IntOr` / `IntXor`, also +/// unconditional, but *int-only*: the float table falls through to +/// `_ => return Ok(None)` for them. Hence the separate +/// `args_all_exact_plain_int`. +/// +/// Every other tag is excluded because its lowering can still decline and +/// leave the residual in place: +/// +/// - `FloorDivide` / `Remainder` (+ in-place) — int-table `needs_concrete_check` +/// declines a zero or `i64::MIN / -1` divisor; the float table has no +/// `FLOAT_*` opcode for either. +/// - `TrueDivide` (+ in-place) — float-table only, and it declines a zero +/// divisor so the raising `descr_truediv` stays recorded. +/// - `Lshift` (+ in-place) — the int table declines it outright (the reused +/// trace would bake a count the x86 `SHL` masks mod 64, and the guarded form +/// breaks the cranelift bridge). +/// - `Rshift` (+ in-place) — declines a negative or `>= LONG_BIT` count rather +/// than baking intobject.py's fold-to-`0`/`-1`. +/// - `Power` (+ in-place) — the int table has no arm; the float table inlines +/// `_pow` but keeps a cold-path residual for nan/inf/negative-base operands. +/// - `Subscr`, `MatrixMultiply` (+ in-place) — no arm in either table. +/// +/// Known limitation: both flags describe the callee's INCOMING arguments, not +/// the operands of the binop itself, which this straight-line scan cannot name. +/// A body can reach a non-numeric operand through the two residuals the scan +/// already treats as replay-safe reads (`LoadConst` / `LoadGlobal`), and a +/// user `__add__` behind one of those would be a live-heap effect the claim +/// misses. The in-place tags do not widen that hole: an in-place result must +/// be stored back, and every store target outside the callee's own registers +/// (`STORE_GLOBAL` / `STORE_ATTR` / `STORE_SUBSCR`) is itself an unproven +/// residual that fails this scan first. +pub(crate) fn residual_call_is_specialized_plain_numeric_binop( body_code: &[u8], - args_all_numeric: bool, + args_all_exact_numeric: bool, + args_all_exact_plain_int: bool, d: &DecodedOp, num_regs_i: usize, constants_i: &[i64], callee_descr_refs: &[DescrRef], ) -> bool { - if !args_all_numeric + if !args_all_exact_numeric || !matches!( d.key, "residual_call_ir_r/iIRd>r" | "residual_call_ir_i/iIRd>i" | "residual_call_ir_v/iIRd" @@ -2245,7 +2288,7 @@ pub(crate) fn residual_call_is_specialized_plain_int_add( } // `iIR`: funcptr i-reg, then the I-list. The first I-list item is the // BINARY_OP tag. It must be in the callee's immutable constants window; - // a runtime tag could select an in-place or user-defined operation. + // a runtime tag could select an operation outside the accepted set. let Some(&i_len) = body_code.get(d.pc + 2) else { return false; }; @@ -2261,10 +2304,26 @@ pub(crate) fn residual_call_is_specialized_plain_int_add( else { return false; }; - matches!( - pyre_interpreter::runtime_ops::binary_op_from_tag(tag), - Some(pyre_interpreter::bytecode::BinaryOperator::Add) - ) + use pyre_interpreter::bytecode::BinaryOperator; + match pyre_interpreter::runtime_ops::binary_op_from_tag(tag) { + Some( + BinaryOperator::Add + | BinaryOperator::Subtract + | BinaryOperator::Multiply + | BinaryOperator::InplaceAdd + | BinaryOperator::InplaceSubtract + | BinaryOperator::InplaceMultiply, + ) => true, + Some( + BinaryOperator::And + | BinaryOperator::Or + | BinaryOperator::Xor + | BinaryOperator::InplaceAnd + | BinaryOperator::InplaceOr + | BinaryOperator::InplaceXor, + ) => args_all_exact_plain_int, + _ => false, + } } pub(crate) fn dispatch_residual_call_iRd_kind( @@ -2888,9 +2947,31 @@ pub(crate) fn dispatch_residual_call_iRd_kind( .profiler() .count_ops(call_opcode, majit_metainterp::counters::RECORDED_OPS); } - let recorded = ctx - .trace_ctx - .record_op_with_descr(call_opcode, &allboxes, descr.clone()); + // `list_write_barrier` on the Object strategy's in-place append arm: + // the backend GC rewrite already marks the same store's items block + // with `COND_CALL_GC_WB_ARRAY`, and the list's `items` pointer did not + // change, so a recorded barrier call is a second barrier upstream never + // emits. Skip the record; the executor below still runs it concretely, + // because the walk itself mutates the live heap. `OpRef::NONE` is safe + // as the result slot: the barrier is void, so the only consumer + // (`set_opref_concrete` on the executed result) is the `Type::Void` + // no-op arm. + let wb_covered = ctx + .fbw_mode + .append_inplace_wb_covered_receiver + .is_some_and(|receiver| { + allboxes.len() == 2 + && matches!(ctx.trace_ctx.box_value(allboxes[0]), Some(majit_ir::Value::Int(addr)) + if pyre_interpreter::is_list_write_barrier(addr as usize)) + && matches!(ctx.trace_ctx.box_value(allboxes[1]), Some(majit_ir::Value::Ref(r)) + if r.as_usize() == receiver) + }); + let recorded = if wb_covered { + OpRef::NONE + } else { + ctx.trace_ctx + .record_op_with_descr(call_opcode, &allboxes, descr.clone()) + }; // pyjitpl.py `_record_helper_pure` parity: for // `CallPure*` whose every argbox carries a known `box_value`, diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs index a1893859e4a..f2cf333fdca 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs @@ -4406,6 +4406,11 @@ pub(crate) fn orthodox_list_append_commit( let value_concrete = ConcreteValue::Ref(value); let saved_fbw_mode = ctx.fbw_mode; ctx.fbw_mode.inline_subwalk = true; + // Read the arm AFTER any empty-strategy promotion above, so the predicate + // sees the storage the sub-walk will actually append into. + ctx.fbw_mode.append_inplace_wb_covered_receiver = + unsafe { pyre_object::w_list_append_stores_into_gc_block_in_place(inner_self) } + .then_some(inner_self as usize); let walk_result = run_sub_jitcode_walk( ctx, op.pc, diff --git a/pyre/pyre-object/src/listobject.rs b/pyre/pyre-object/src/listobject.rs index e36c6bd484c..2d62cf5af34 100644 --- a/pyre/pyre-object/src/listobject.rs +++ b/pyre/pyre-object/src/listobject.rs @@ -1031,6 +1031,27 @@ pub unsafe fn w_list_uses_empty_storage(obj: PyObjectRef) -> bool { list.strategy == ListStrategy::Empty } +/// True when the next `w_list_append` on `obj` takes the Object strategy's +/// in-place arm (`rlist.py:285` resize-ge fast case) into a GC-managed items +/// block: spare capacity, so the store is an item-slot write and the list's +/// `items` pointer does not change. +/// +/// The tracer uses this to decide whether the recorded trace needs +/// `list_write_barrier` at all — see `FbwWalkMode::append_inplace_wb_covered`. +/// A `std::alloc` block (`PYRE_GC_ITEMSBLOCK=0`) answers false: it has no GC +/// header for an array barrier to mark, so the barrier on the enclosing +/// `W_ListObject` is the only thing keeping the block's slots reachable. +/// +/// # Safety +/// `obj` must point to a valid `W_ListObject`. +pub unsafe fn w_list_append_stores_into_gc_block_in_place(obj: PyObjectRef) -> bool { + let list = &*(obj as *const W_ListObject); + list.strategy == ListStrategy::Object + && ll_list_obj_length(list) < ll_list_obj_capacity(list) + && !list.items.is_null() + && crate::gc_hook::try_gc_owns_object(list.items as *mut u8) +} + /// Rebuild the list's object storage from a Vec. unsafe fn rebuild_object_items(list: &mut W_ListObject, items: Vec) { list.set_object_items_from_vec(items);