From 738e10aaddc2cf2829de6a43da9e0e8454cb3ed8 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 5 Aug 2026 06:49:27 +0900 Subject: [PATCH 01/10] gc, jit: bind the arity-2 specialised tuple types to their own vtable entries `Cls_ii` / `Cls_ff` / `Cls_oo` each carry their own `ob_type` and their typeids already sit in `SUBCLASS_RANGE_HIERARCHY`, but no `register_vtable_for_type` bound those PyTypes to those typeids. `subclass_range` therefore answered "unknown" for a specialised tuple, so `protect_speculative_field` rejected any pure field fold on a constant one and the optimizer raised `InvalidLoop` for the whole trace instead of declining the fold. Register the three vtables and add the matching entries to the alias census the registration is asserted against. Assisted-by: Claude --- pyre/pyre-jit/src/eval.rs | 22 ++++++++++++++++++++++ pyre/pyre-object/src/pyobject.rs | 18 ++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index 3bfcd775aa9..bfb3176f254 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -1678,6 +1678,28 @@ fn build_gc() -> Box { &pyre_object::pyobject::TUPLE_TYPE as *const _ as usize, w_tuple_tid, ); + // Each arity-2 specialisation gets its own `ob_type`, so bind each to its + // own typeid. `cls_of_gcref` reads that `ob_type`, and without a binding + // `subclass_range` cannot answer for it — which makes + // `protect_speculative_field` reject any pure field fold on a constant + // specialised tuple and invalidate the loop rather than decline the fold. + for (pytype_ptr, tid) in [ + ( + &pyre_object::specialisedtupleobject::SPECIALISED_TUPLE_II_TYPE as *const _ as usize, + spec_tuple_ii_tid, + ), + ( + &pyre_object::specialisedtupleobject::SPECIALISED_TUPLE_FF_TYPE as *const _ as usize, + spec_tuple_ff_tid, + ), + ( + &pyre_object::specialisedtupleobject::SPECIALISED_TUPLE_OO_TYPE as *const _ as usize, + spec_tuple_oo_tid, + ), + ] { + majit_gc::GcAllocator::register_vtable_for_type(&mut gc, pytype_ptr, tid); + pytype_to_tid.insert(pytype_ptr, tid); + } // BuiltinCode is pre-registered (rather than picked up by the // foreign-pytype loop below) because the loop hard-codes // `size_of::()` as the payload size, while the diff --git a/pyre/pyre-object/src/pyobject.rs b/pyre/pyre-object/src/pyobject.rs index 3af2c23df1f..7a8923b55b8 100644 --- a/pyre/pyre-object/src/pyobject.rs +++ b/pyre/pyre-object/src/pyobject.rs @@ -1022,6 +1022,24 @@ pub fn all_subclass_range_aliases() -> Vec { subclass_range_alias(6, &crate::functional::RANGE_ITER_TYPE), subclass_range_alias(7, &LIST_TYPE), subclass_range_alias(8, &TUPLE_TYPE), + // The three arity-2 specialisations each carry their own `ob_type` + // (`specialisedtupleobject.py` `Cls_ii / Cls_ff / Cls_oo`), so they + // need their own vtable binding: without one, `subclass_range` on a + // specialised tuple answers "unknown" and every pure field fold on a + // constant one — `f.__defaults__` pinned by an identity guard, say — + // fails `protect_speculative_field` and invalidates the whole loop. + subclass_range_alias( + 10, + &crate::specialisedtupleobject::SPECIALISED_TUPLE_II_TYPE, + ), + subclass_range_alias( + 11, + &crate::specialisedtupleobject::SPECIALISED_TUPLE_FF_TYPE, + ), + subclass_range_alias( + 12, + &crate::specialisedtupleobject::SPECIALISED_TUPLE_OO_TYPE, + ), subclass_range_alias(15, &crate::nestedscope::CELL_TYPE), subclass_range_alias(16, &crate::function::METHOD_TYPE), subclass_range_alias(17, &crate::sliceobject::SLICE_TYPE), From f28eb9aa849962078e4cd125300dc8831e723e72 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 5 Aug 2026 06:49:40 +0900 Subject: [PATCH 02/10] jit: inline calls that fill a parameter from defs_w through a specialised defaults tuple or a keyword hole `w_tuple_new` routes every arity-2 tuple through `makespecialisedtuple2`, so a callee with exactly two defaulted parameters carries a `W_SpecialisedTupleObject_*` as `__defaults__`. `positional_defaults_for_inline` accepted the array-backed `wrappeditems` layout only, so such a call stayed a residual. Read `Cls_ii` through its unboxed `value0` / `value1` plus `wrapint` and `Cls_oo` through its object slots; `Cls_ff` has no walker float-field read to pair with `wrapfloat` and stays residual. `fbw_reorder_call_kw_args` separately required every parameter to be filled from a passed argument, so a keyword call that left one to its default stayed residual too. It now leaves a hole, and the default filling parameter `p` is picked as `defs_w[p - (co_argcount - len(defs_w))]` instead of taking the tail of `defs_w`, so a hole anywhere in the parameter list binds. `synth/calls_closures` runs at median 0.551 of its previous wall clock over 9 pairwise rounds, against a 0.96-1.05 self-A/B band; its `default_keyword_args` section goes 0.163s -> 0.018s of execution-only time, and measured on its own that loop runs at median 0.282. Assisted-by: Claude --- .../parity_tests/call_defaults_inline.py | 137 ++++++++++++ .../src/jitcode_dispatch/inline_call.rs | 203 +++++++++++++----- 2 files changed, 288 insertions(+), 52 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/call_defaults_inline.py diff --git a/pyre/extra_tests/parity_tests/call_defaults_inline.py b/pyre/extra_tests/parity_tests/call_defaults_inline.py new file mode 100644 index 00000000000..01f0d69af96 --- /dev/null +++ b/pyre/extra_tests/parity_tests/call_defaults_inline.py @@ -0,0 +1,137 @@ +# Argument binding for calls that leave parameters to their defaults, run hot +# enough to be traced. +# +# `w_tuple_new` routes every arity-2 tuple through `makespecialisedtuple2`, so +# `__defaults__` for a two-defaulted signature is a `W_SpecialisedTupleObject_*` +# rather than the array-backed layout — a different element read, and for the +# int pair an unboxed one. A keyword argument can also leave a hole anywhere in +# the parameter list rather than a missing tail, so the default that fills slot +# `p` is `defs_w[p - (co_argcount - len(defs_w))]`, not "the tail of defs_w". +# +# Every case below is a fold whose result depends on each parameter, so a +# default read from the wrong slot changes the number. + +N = 300 + + +def check(got, want, label): + assert got == want, "%s: %r != %r" % (label, got, want) + + +# ── one default: array-backed `defs_w` ── +def one(a, b=3): + return a * 100 + b + + +# ── two defaults: the int pair, `Cls_ii`, unboxed slots ── +def two_ints(a, b=3, c=5): + return a * 100 + b * 10 + c + + +# ── two defaults that are not both plain ints: `Cls_oo`, object slots ── +def two_objs(a, b="x", c=None): + return "%d-%s-%s" % (a, b, c) + + +def two_bools(a, b=True, c=False): + # `is_plain_int1` rejects bool, so this pair is `Cls_oo`, not `Cls_ii`. + return a * 100 + (10 if b else 0) + (1 if c else 0) + + +# ── two float defaults: `Cls_ff`, which stays on the residual path ── +def two_floats(a, b=0.5, c=0.25): + return a + b * 10 + c * 100 + + +# ── three defaults: array-backed again ── +def three(a, b=3, c=5, d=7): + return a * 1000 + b * 100 + c * 10 + d + + +# ── a default the callee mutates: the SAME object every call ── +def accumulating(a, acc=[]): + acc.append(a) + return len(acc) + + +# ── positional-only: a keyword may not bind `a` ── +def posonly(a, /, b=3, c=5): + return a * 100 + b * 10 + c + + +for i in range(N): + check(one(1), 103, "one/default") + check(one(1, 4), 104, "one/positional") + check(one(1, b=4), 104, "one/keyword") + + check(two_ints(1), 135, "ii/both defaults") + check(two_ints(1, 4), 145, "ii/one positional") + check(two_ints(1, 4, 6), 146, "ii/none defaulted") + # a keyword leaving a HOLE at `b` rather than a missing tail + check(two_ints(1, c=7), 137, "ii/hole at b") + check(two_ints(1, b=4), 145, "ii/keyword b") + check(two_ints(1, b=4, c=6), 146, "ii/both keyword") + check(two_ints(1, c=6, b=4), 146, "ii/both keyword reordered") + + check(two_objs(1), "1-x-None", "oo/both defaults") + check(two_objs(1, "y"), "1-y-None", "oo/one positional") + check(two_objs(1, c="z"), "1-x-z", "oo/hole at b") + + check(two_bools(1), 110, "bool/both defaults") + check(two_bools(1, c=True), 111, "bool/hole at b") + + check(two_floats(1.0), 31.0, "ff/both defaults") + check(two_floats(1.0, c=0.5), 56.0, "ff/hole at b") + + check(three(1), 1357, "three/all defaults") + check(three(1, d=8), 1358, "three/hole at b,c") + check(three(1, 4, d=8), 1458, "three/hole at c") + + check(posonly(1), 135, "posonly/defaults") + check(posonly(1, c=7), 137, "posonly/hole at b") + + check(accumulating(i), i + 1, "shared mutable default") + +# A keyword aimed at a positional-only parameter is a TypeError, inlined or not. +try: + posonly(a=1) +except TypeError: + pass +else: + raise AssertionError("posonly(a=1) must raise TypeError") + +# Missing a parameter that has no default is a TypeError too. +def needs_two(a, b): + return a + b + + +try: + needs_two(1) +except TypeError: + pass +else: + raise AssertionError("needs_two(1) must raise TypeError") + + +# `__defaults__` replaced mid-loop must be observed: the tuple identity the +# trace pinned is gone, so binding has to re-derive which element fills which +# parameter — including when the replacement changes the tuple's length AND its +# representation (3-tuple array-backed -> 2-int pair). +def swapped(a, b=3, c=5): + return a * 100 + b * 10 + c + + +seen = [] +for i in range(N): + if i == N // 3: + swapped.__defaults__ = (4, 6) + elif i == 2 * N // 3: + swapped.__defaults__ = (7, 8, 9) + seen.append(swapped(1)) +check(seen[0], 135, "swap/before") +check(seen[N // 3], 146, "swap/after two-int pair") +# A defaults tuple LONGER than the parameter list keeps its tail: `def_first` +# goes negative and `b`/`c` take `defs_w[1]` / `defs_w[2]`. +check(seen[2 * N // 3], 189, "swap/after over-long tuple") + +print("OK") 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 906e1c2d297..f496eeece38 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -23,48 +23,87 @@ struct BoundMethodInline { receiver: pyre_object::PyObjectRef, } +/// Where an element of a `defs_w` tuple lives, and therefore what the trace +/// emits to read one. `w_tuple_new` routes EVERY arity-2 tuple through +/// `makespecialisedtuple2` (`specialisedtupleobject.py:169-179`), so a callee +/// with exactly two defaulted parameters — an extremely ordinary signature — +/// never has an array-backed `defs_w` at all. +#[derive(Clone, Copy, PartialEq, Eq)] +enum DefaultsRepr { + /// Array-backed `W_TupleObject`: the elements live in the `wrappeditems` + /// block, the shape upstream's `defs_w?[*]` lowers to. + ItemsBlock, + /// `Cls_ii`: two inline machine ints. `wraps[i]` is `wrapint` + /// (`specialisedtupleobject.py:138-141`), which is what + /// `_flat_pycall_defaults` already runs per call through + /// `w_tuple_getitem`, so the emitted box is the same fresh box the + /// interpreter would have made. + PairInt, + /// `Cls_oo`: two inline object slots, for which `wraps[i]` is the identity + /// (`specialisedtupleobject.py:26-27`) — the field read IS the element. + PairObject, +} + struct PositionalDefaultsInline { tuple: pyre_object::PyObjectRef, + repr: DefaultsRepr, /// `(parameter index, tuple index, concrete value)`. values: Vec<(usize, usize, pyre_object::PyObjectRef)>, } -/// `function.py:188-193,217-231` — determine the tail of `defs_w` used by a -/// flat positional call with missing arguments. The trace currently accepts -/// the ordinary translated `W_TupleObject` representation only: its -/// `wrappeditems[*]` storage has the exact immutable-array shape upstream's -/// `defs_w?[*]` lowers to. Specialised numeric tuples are a different -/// unboxed representation and safely remain on the residual call path. +/// `function.py:188-193,217-231` — which `defs_w` element fills each parameter +/// the call left unbound. `defs_w` covers the LAST `len(defs_w)` parameters, +/// so parameter `p` takes `defs_w[p - (nparams - ndefaults)]`; a `missing` +/// parameter below that floor has no default and the call would raise, so the +/// whole inline declines. +/// +/// `Cls_ff` is left out: its slots are inline `f64` and the walker has no +/// float-field read to pair with `wrapfloat` here, so a two-float defaults +/// tuple safely stays on the residual call path. unsafe fn positional_defaults_for_inline( callable: pyre_object::PyObjectRef, - nargs: usize, + missing: &[usize], nparams: usize, ) -> Option { - if nargs >= nparams { + if missing.is_empty() { return None; } let tuple = unsafe { pyre_interpreter::function_get_defaults(callable) }; - if tuple.is_null() - || !std::ptr::eq( - unsafe { (*tuple).ob_type }, - &pyre_object::pyobject::TUPLE_TYPE, - ) - { + if tuple.is_null() { return None; } - let ndefaults = unsafe { pyre_object::w_tuple_len(tuple) }; - let missing = nparams - nargs; - if missing > ndefaults { + // The layout is what `ob_type` names, and the identity guard the emitting + // half records pins this exact object — so the type test here decides + // which read to emit, and nothing later can invalidate it. + let ob_type = unsafe { (*tuple).ob_type }; + let repr = if std::ptr::eq(ob_type, &pyre_object::pyobject::TUPLE_TYPE) { + DefaultsRepr::ItemsBlock + } else if std::ptr::eq( + ob_type, + &pyre_object::specialisedtupleobject::SPECIALISED_TUPLE_II_TYPE, + ) { + DefaultsRepr::PairInt + } else if std::ptr::eq( + ob_type, + &pyre_object::specialisedtupleobject::SPECIALISED_TUPLE_OO_TYPE, + ) { + DefaultsRepr::PairObject + } else { return None; - } - let start = ndefaults - missing; - let mut values = Vec::with_capacity(missing); - for offset in 0..missing { - let tuple_index = start + offset; + }; + let ndefaults = unsafe { pyre_object::w_tuple_len(tuple) }; + let first_defaulted = nparams.checked_sub(ndefaults)?; + let mut values = Vec::with_capacity(missing.len()); + for ¶m_index in missing { + let tuple_index = param_index.checked_sub(first_defaulted)?; let value = unsafe { pyre_object::w_tuple_getitem(tuple, tuple_index as i64) }?; - values.push((nargs + offset, tuple_index, value)); + values.push((param_index, tuple_index, value)); } - Some(PositionalDefaultsInline { tuple, values }) + Some(PositionalDefaultsInline { + tuple, + repr, + values, + }) } /// Path-1 (#68): resolve a scalar `getfield_vable_r` read off an inlined @@ -1381,11 +1420,14 @@ unsafe fn fbw_reorder_call_kw_args( return None; } let nkw = unsafe { pyre_object::w_tuple_len(kwnames) }; - // Every positional parameter must be filled exactly once from a passed arg: - // no defaults, no *args/**kwargs/keyword-only slots the seeding would leave - // unbound. + // No positional parameter may be filled more than once, and the call may + // not pass more than the callee takes — `*args` / `**kwargs` / + // keyword-only slots are ruled out separately by + // `fbw_callee_scope_is_positional_only`. A parameter left unbound is + // allowed through as a hole; the caller fills it from `defs_w` or declines + // when it has no default. let receiver_count = usize::from(receiver.is_some()); - if nparams == 0 || nkw > nargs || nargs + receiver_count != nparams { + if nparams == 0 || nkw > nargs || nargs + receiver_count > nparams { return None; } let raw = unsafe { @@ -1442,8 +1484,8 @@ unsafe fn fbw_reorder_call_kw_args( let mut out_args = Vec::with_capacity(nparams); let mut out_conc = Vec::with_capacity(nparams); for k in 0..nparams { - out_args.push(slot_args[k]?); - out_conc.push(slot_conc[k]?); + out_args.push(slot_args[k].unwrap_or(OpRef::NONE)); + out_conc.push(slot_conc[k].unwrap_or(ConcreteValue::Null)); } Some((out_args, out_conc)) } @@ -2567,23 +2609,36 @@ pub(crate) fn try_walker_inline_resolved_user_call( if !fbw_callee_scope_is_positional_only(w_code) { return Ok(None); } - // `Function.funccall_valuestack` fills a missing positional tail from - // `defs_w` before entering the frame (`function.py:188-193,217-231`). - // Mirror that frame shape here. Placeholder boxes are replaced by live - // guarded tuple-item reads after all non-emitting eligibility checks. - let positional_defaults = if callee_args.len() < nparams { + // `Function.funccall_valuestack` fills every parameter the call left + // unbound from `defs_w` before entering the frame + // (`function.py:188-193,217-231`); `Arguments.parse` reaches the same frame + // shape for a keyword call. Mirror that shape here. Placeholder boxes are + // replaced by live guarded tuple-item reads after all non-emitting + // eligibility checks. + // + // A positional call leaves the unbound parameters as a missing tail, but a + // keyword one can leave a hole anywhere — `f(i, c=7)` on `def f(a, b=3, + // c=5)` binds slots 0 and 2 and leaves 1 — so the seeding vectors carry + // `OpRef::NONE` for a hole and the set is collected rather than assumed + // contiguous. Only the seeding writes that sentinel, so it cannot collide + // with a genuinely passed argument. + let missing: Vec = (0..nparams) + .filter(|&i| callee_args.get(i).is_none_or(|arg| *arg == OpRef::NONE)) + .collect(); + let positional_defaults = if missing.is_empty() { + None + } else { let Some(defaults) = - (unsafe { positional_defaults_for_inline(callable, callee_args.len(), nparams) }) + (unsafe { positional_defaults_for_inline(callable, &missing, nparams) }) else { return Ok(None); }; - for &(_, _, value) in &defaults.values { - callee_args.push(OpRef::NONE); - callee_arg_concretes.push(ConcreteValue::Ref(value)); + callee_args.resize(nparams, OpRef::NONE); + callee_arg_concretes.resize(nparams, ConcreteValue::Null); + for &(param_index, _, value) in &defaults.values { + callee_arg_concretes[param_index] = ConcreteValue::Ref(value); } Some(defaults) - } else { - None }; // Does any incoming binding land a value the callee's register banks can // hold unboxed? Only the `is`-against-None scan below consults this; see @@ -3400,18 +3455,62 @@ pub(crate) fn try_walker_inline_resolved_user_call( .record_guard(OpCode::GuardValue, &[defaults_op, tuple_expected], 0); walker_capture_snapshot_for_last_guard(ctx, op.pc)?; - let items = crate::state::opimpl_getfield_gc_r( - ctx.trace_ctx, - defaults_op, - crate::descr::tuple_wrappeditems_descr(), - ); - // Preserve the actual Ref boxes instead of baking one anchor's - // concrete value. - for (param_index, tuple_index, _) in defaults.values { - let index = ctx.trace_ctx.const_int(tuple_index as i64); - callee_args[param_index] = - crate::state::trace_items_block_getitem_value_pure(ctx.trace_ctx, items, index); + // concrete value. Which read that is depends on where the element + // lives; the identity guard above already proved the layout, so no + // arm needs a class guard of its own. + match defaults.repr { + DefaultsRepr::ItemsBlock => { + let items = crate::state::opimpl_getfield_gc_r( + ctx.trace_ctx, + defaults_op, + crate::descr::tuple_wrappeditems_descr(), + ); + for (param_index, tuple_index, _) in defaults.values { + let index = ctx.trace_ctx.const_int(tuple_index as i64); + callee_args[param_index] = crate::state::trace_items_block_getitem_value_pure( + ctx.trace_ctx, + items, + index, + ); + } + } + DefaultsRepr::PairObject => { + for (param_index, tuple_index, _) in defaults.values { + let descr = if tuple_index == 0 { + crate::descr::specialised_tuple_oo_value0_descr() + } else { + crate::descr::specialised_tuple_oo_value1_descr() + }; + callee_args[param_index] = + crate::state::opimpl_getfield_gc_r(ctx.trace_ctx, defaults_op, descr); + } + } + DefaultsRepr::PairInt => { + for (param_index, tuple_index, value) in defaults.values { + let descr = if tuple_index == 0 { + crate::descr::specialised_tuple_ii_value0_descr() + } else { + crate::descr::specialised_tuple_ii_value1_descr() + }; + let raw = crate::state::opimpl_getfield_gc_i(ctx.trace_ctx, defaults_op, descr); + let elem = unsafe { pyre_object::w_int_get_value(value) }; + let boxed = walker_box_int(ctx, op.pc, raw, elem)?; + // `wrapint` emits a heap `NewWithVtable`, so the stamped + // concrete has to be a heap pointer too — the walk-time + // `w_tuple_getitem` box above may be a tagged immediate. + // Re-home the argument's concrete onto whatever + // `box_int_concrete` picked, or the seeding loop below + // would re-stamp the op with the other one. + let concrete = box_int_concrete(elem, value as i64); + if let majit_ir::Value::Ref(gcref) = concrete { + callee_arg_concretes[param_index] = + ConcreteValue::Ref(gcref.as_usize() as pyre_object::PyObjectRef); + } + ctx.trace_ctx.set_opref_concrete(boxed, concrete); + callee_args[param_index] = boxed; + } + } } } From b5bf3d37e39b7f478a9deae8e68abafa246a659a Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 5 Aug 2026 08:44:27 +0900 Subject: [PATCH 03/10] jit: make the list write barrier's root bracket an opaque boundary so object-strategy appends fold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `prepare_list_ref_store` wraps `list_write_barrier` in a `push_roots` bracket so the stored value survives the safepoint inside the barrier's ownership query. The bracket's zero-arg root-stack resolve has no registered fnaddr, so when the orthodox `w_list_append` descent reached it the call carried a `symbolic_fnaddr_for_path` hash and `try_execute_residual_call_via_executor` declined the whole sub-walk. Every Object-strategy append therefore fell back to the generic residual — in both the `lst.append(x)` method form and the LIST_APPEND comprehension form — while the Integer arm, which has no bracket, folded. Mark `prepare_list_ref_store` `dont_look_inside` and register it, and extend `is_list_write_barrier` to match the wrapper so the residual keeps its exemption from the FBW body-effect accounting. The Object arm now records `guard_value(strategy)` + `getfield(items)` + `arraylen_gc` + `setarrayitem_gc` + `setfield(length)` with the barrier as its one residual, in place of a `Method` allocation plus `CallMayForce`. `synth/list_ops` `list_obj_append_pop` exec 0.179s -> 0.112s (19.9x -> 11.1x pypy); the merged file's wall clock A/B median is 0.823 over 9 pairs against a 0.998 self band. The new capacity and strategy guards are visible in jit-stats: 16 patterns re-recorded for `bridges_compiled` / `guard_failures`, and `loops_compiled` is unchanged everywhere. Most deltas match on dynasm and cranelift; `pickle_terminal_raise_resume` does not — cranelift 664 -> 668 against dynasm 450 -> 482 — and its pre-fold values already differed between the two backends. New fixture `parity_tests/list_object_append_fold.py`. Assisted-by: Claude --- ...nsion_object_append_hot.cranelift.jitstats | 4 +- ...ehension_object_append_hot.dynasm.jitstats | 4 +- .../const_arg_call_resume.cranelift.jitstats | 4 +- .../const_arg_call_resume.dynasm.jitstats | 4 +- ..._catching_frame_tb_node.cranelift.jitstats | 2 +- ...ion_catching_frame_tb_node.dynasm.jitstats | 2 +- ...inline_callee_tb_frames.cranelift.jitstats | 4 +- ...on_inline_callee_tb_frames.dynasm.jitstats | 4 +- ..._guard_finally_residual.cranelift.jitstats | 4 +- ...try_guard_finally_residual.dynasm.jitstats | 4 +- ..._traceback_frame_lineno.cranelift.jitstats | 2 +- ...ion_traceback_frame_lineno.dynasm.jitstats | 2 +- ..._traceback_lineno_chain.cranelift.jitstats | 4 +- ...ion_traceback_lineno_chain.dynasm.jitstats | 4 +- ..._flavor_traceback_names.cranelift.jitstats | 4 +- ...dge_flavor_traceback_names.dynasm.jitstats | 4 +- ...append_write_barrier_gc.cranelift.jitstats | 2 +- ...st_append_write_barrier_gc.dynasm.jitstats | 2 +- .../minmax_key_rooting.cranelift.jitstats | 2 +- .../synth/minmax_key_rooting.dynasm.jitstats | 2 +- ..._list_comprehension_hot.cranelift.jitstats | 4 +- ...ted_list_comprehension_hot.dynasm.jitstats | 4 +- ...e_terminal_raise_resume.cranelift.jitstats | 2 +- ...ckle_terminal_raise_resume.dynasm.jitstats | 2 +- .../sre_pattern_methods.cranelift.jitstats | 4 +- .../synth/sre_pattern_methods.dynasm.jitstats | 4 +- .../synth/sre_wasm_min.cranelift.jitstats | 4 +- pyre/bench/synth/sre_wasm_min.dynasm.jitstats | 4 +- .../synth/sre_wasm_min1.cranelift.jitstats | 4 +- .../bench/synth/sre_wasm_min1.dynasm.jitstats | 4 +- ...ndex_bytes_iter_surface.cranelift.jitstats | 2 +- ...r_index_bytes_iter_surface.dynasm.jitstats | 2 +- .../parity_tests/list_object_append_fold.py | 201 ++++++++++++++++++ pyre/pyre-interpreter/src/jit_fnaddr.rs | 26 ++- pyre/pyre-object/src/listobject.rs | 17 +- 35 files changed, 294 insertions(+), 54 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/list_object_append_fold.py diff --git a/pyre/bench/synth/comprehension_object_append_hot.cranelift.jitstats b/pyre/bench/synth/comprehension_object_append_hot.cranelift.jitstats index b845ac1b8ba..86d936cfe36 100644 --- a/pyre/bench/synth/comprehension_object_append_hot.cranelift.jitstats +++ b/pyre/bench/synth/comprehension_object_append_hot.cranelift.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=8 +bridges_compiled=18 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1605 +guard_failures=3612 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/comprehension_object_append_hot.dynasm.jitstats b/pyre/bench/synth/comprehension_object_append_hot.dynasm.jitstats index b845ac1b8ba..86d936cfe36 100644 --- a/pyre/bench/synth/comprehension_object_append_hot.dynasm.jitstats +++ b/pyre/bench/synth/comprehension_object_append_hot.dynasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=8 +bridges_compiled=18 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1605 +guard_failures=3612 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/const_arg_call_resume.cranelift.jitstats b/pyre/bench/synth/const_arg_call_resume.cranelift.jitstats index 255c4bc61e2..f7e39b7e145 100644 --- a/pyre/bench/synth/const_arg_call_resume.cranelift.jitstats +++ b/pyre/bench/synth/const_arg_call_resume.cranelift.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=7 +bridges_compiled=9 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1403 +guard_failures=1804 internal_compile_panics=0 loops_aborted=0 loops_compiled=3 diff --git a/pyre/bench/synth/const_arg_call_resume.dynasm.jitstats b/pyre/bench/synth/const_arg_call_resume.dynasm.jitstats index 255c4bc61e2..f7e39b7e145 100644 --- a/pyre/bench/synth/const_arg_call_resume.dynasm.jitstats +++ b/pyre/bench/synth/const_arg_call_resume.dynasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=7 +bridges_compiled=9 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1403 +guard_failures=1804 internal_compile_panics=0 loops_aborted=0 loops_compiled=3 diff --git a/pyre/bench/synth/exception_catching_frame_tb_node.cranelift.jitstats b/pyre/bench/synth/exception_catching_frame_tb_node.cranelift.jitstats index 5c3c7c0c314..e77bee0a52a 100644 --- a/pyre/bench/synth/exception_catching_frame_tb_node.cranelift.jitstats +++ b/pyre/bench/synth/exception_catching_frame_tb_node.cranelift.jitstats @@ -1,4 +1,4 @@ -bridges_compiled=2 +bridges_compiled=3 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 diff --git a/pyre/bench/synth/exception_catching_frame_tb_node.dynasm.jitstats b/pyre/bench/synth/exception_catching_frame_tb_node.dynasm.jitstats index 5c3c7c0c314..e77bee0a52a 100644 --- a/pyre/bench/synth/exception_catching_frame_tb_node.dynasm.jitstats +++ b/pyre/bench/synth/exception_catching_frame_tb_node.dynasm.jitstats @@ -1,4 +1,4 @@ -bridges_compiled=2 +bridges_compiled=3 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 diff --git a/pyre/bench/synth/exception_inline_callee_tb_frames.cranelift.jitstats b/pyre/bench/synth/exception_inline_callee_tb_frames.cranelift.jitstats index 3c88558b470..4ceb46593e9 100644 --- a/pyre/bench/synth/exception_inline_callee_tb_frames.cranelift.jitstats +++ b/pyre/bench/synth/exception_inline_callee_tb_frames.cranelift.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=2 +bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=406 +guard_failures=603 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/exception_inline_callee_tb_frames.dynasm.jitstats b/pyre/bench/synth/exception_inline_callee_tb_frames.dynasm.jitstats index 3c88558b470..4ceb46593e9 100644 --- a/pyre/bench/synth/exception_inline_callee_tb_frames.dynasm.jitstats +++ b/pyre/bench/synth/exception_inline_callee_tb_frames.dynasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=2 +bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=406 +guard_failures=603 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/exception_reentry_guard_finally_residual.cranelift.jitstats b/pyre/bench/synth/exception_reentry_guard_finally_residual.cranelift.jitstats index cc13e71d123..97cc4a44a37 100644 --- a/pyre/bench/synth/exception_reentry_guard_finally_residual.cranelift.jitstats +++ b/pyre/bench/synth/exception_reentry_guard_finally_residual.cranelift.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=11 +bridges_compiled=12 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=2261 +guard_failures=2461 internal_compile_panics=0 loops_aborted=0 loops_compiled=3 diff --git a/pyre/bench/synth/exception_reentry_guard_finally_residual.dynasm.jitstats b/pyre/bench/synth/exception_reentry_guard_finally_residual.dynasm.jitstats index cc13e71d123..97cc4a44a37 100644 --- a/pyre/bench/synth/exception_reentry_guard_finally_residual.dynasm.jitstats +++ b/pyre/bench/synth/exception_reentry_guard_finally_residual.dynasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=11 +bridges_compiled=12 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=2261 +guard_failures=2461 internal_compile_panics=0 loops_aborted=0 loops_compiled=3 diff --git a/pyre/bench/synth/exception_traceback_frame_lineno.cranelift.jitstats b/pyre/bench/synth/exception_traceback_frame_lineno.cranelift.jitstats index 2fb55a7c717..38e4fbc5545 100644 --- a/pyre/bench/synth/exception_traceback_frame_lineno.cranelift.jitstats +++ b/pyre/bench/synth/exception_traceback_frame_lineno.cranelift.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=816 +guard_failures=812 internal_compile_panics=0 loops_aborted=0 loops_compiled=17 diff --git a/pyre/bench/synth/exception_traceback_frame_lineno.dynasm.jitstats b/pyre/bench/synth/exception_traceback_frame_lineno.dynasm.jitstats index 2fb55a7c717..38e4fbc5545 100644 --- a/pyre/bench/synth/exception_traceback_frame_lineno.dynasm.jitstats +++ b/pyre/bench/synth/exception_traceback_frame_lineno.dynasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=816 +guard_failures=812 internal_compile_panics=0 loops_aborted=0 loops_compiled=17 diff --git a/pyre/bench/synth/exception_traceback_lineno_chain.cranelift.jitstats b/pyre/bench/synth/exception_traceback_lineno_chain.cranelift.jitstats index c98a602a429..b4b631c02bf 100644 --- a/pyre/bench/synth/exception_traceback_lineno_chain.cranelift.jitstats +++ b/pyre/bench/synth/exception_traceback_lineno_chain.cranelift.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=2 +bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=404 +guard_failures=605 internal_compile_panics=0 loops_aborted=0 loops_compiled=5 diff --git a/pyre/bench/synth/exception_traceback_lineno_chain.dynasm.jitstats b/pyre/bench/synth/exception_traceback_lineno_chain.dynasm.jitstats index c98a602a429..b4b631c02bf 100644 --- a/pyre/bench/synth/exception_traceback_lineno_chain.dynasm.jitstats +++ b/pyre/bench/synth/exception_traceback_lineno_chain.dynasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=2 +bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=404 +guard_failures=605 internal_compile_panics=0 loops_aborted=0 loops_compiled=5 diff --git a/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.cranelift.jitstats b/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.cranelift.jitstats index 78e703f1c08..66923ce27ec 100644 --- a/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.cranelift.jitstats +++ b/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.cranelift.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=7 +bridges_compiled=8 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1654 +guard_failures=1671 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.dynasm.jitstats b/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.dynasm.jitstats index 78e703f1c08..66923ce27ec 100644 --- a/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.dynasm.jitstats +++ b/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.dynasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=7 +bridges_compiled=8 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1654 +guard_failures=1671 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/list_append_write_barrier_gc.cranelift.jitstats b/pyre/bench/synth/list_append_write_barrier_gc.cranelift.jitstats index 84a1bea3f6a..478af581564 100644 --- a/pyre/bench/synth/list_append_write_barrier_gc.cranelift.jitstats +++ b/pyre/bench/synth/list_append_write_barrier_gc.cranelift.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1335 +guard_failures=1345 internal_compile_panics=0 loops_aborted=1 loops_compiled=12 diff --git a/pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats b/pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats index 84a1bea3f6a..478af581564 100644 --- a/pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats +++ b/pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1335 +guard_failures=1345 internal_compile_panics=0 loops_aborted=1 loops_compiled=12 diff --git a/pyre/bench/synth/minmax_key_rooting.cranelift.jitstats b/pyre/bench/synth/minmax_key_rooting.cranelift.jitstats index 8beed56f050..ce08616f8db 100644 --- a/pyre/bench/synth/minmax_key_rooting.cranelift.jitstats +++ b/pyre/bench/synth/minmax_key_rooting.cranelift.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1 +guard_failures=5 internal_compile_panics=0 loops_aborted=0 loops_compiled=1 diff --git a/pyre/bench/synth/minmax_key_rooting.dynasm.jitstats b/pyre/bench/synth/minmax_key_rooting.dynasm.jitstats index 8beed56f050..ce08616f8db 100644 --- a/pyre/bench/synth/minmax_key_rooting.dynasm.jitstats +++ b/pyre/bench/synth/minmax_key_rooting.dynasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1 +guard_failures=5 internal_compile_panics=0 loops_aborted=0 loops_compiled=1 diff --git a/pyre/bench/synth/nested_list_comprehension_hot.cranelift.jitstats b/pyre/bench/synth/nested_list_comprehension_hot.cranelift.jitstats index f4509ffc3bb..ee7d2607be7 100644 --- a/pyre/bench/synth/nested_list_comprehension_hot.cranelift.jitstats +++ b/pyre/bench/synth/nested_list_comprehension_hot.cranelift.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=2 +bridges_compiled=6 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=401 +guard_failures=1204 internal_compile_panics=0 loops_aborted=0 loops_compiled=2 diff --git a/pyre/bench/synth/nested_list_comprehension_hot.dynasm.jitstats b/pyre/bench/synth/nested_list_comprehension_hot.dynasm.jitstats index f4509ffc3bb..ee7d2607be7 100644 --- a/pyre/bench/synth/nested_list_comprehension_hot.dynasm.jitstats +++ b/pyre/bench/synth/nested_list_comprehension_hot.dynasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=2 +bridges_compiled=6 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=401 +guard_failures=1204 internal_compile_panics=0 loops_aborted=0 loops_compiled=2 diff --git a/pyre/bench/synth/pickle_terminal_raise_resume.cranelift.jitstats b/pyre/bench/synth/pickle_terminal_raise_resume.cranelift.jitstats index a0ae1bf3c57..4ce9dd1e102 100644 --- a/pyre/bench/synth/pickle_terminal_raise_resume.cranelift.jitstats +++ b/pyre/bench/synth/pickle_terminal_raise_resume.cranelift.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=652 +guard_failures=668 internal_compile_panics=0 loops_aborted=1 loops_compiled=36 diff --git a/pyre/bench/synth/pickle_terminal_raise_resume.dynasm.jitstats b/pyre/bench/synth/pickle_terminal_raise_resume.dynasm.jitstats index 9b26cc29c2d..f39e9177c47 100644 --- a/pyre/bench/synth/pickle_terminal_raise_resume.dynasm.jitstats +++ b/pyre/bench/synth/pickle_terminal_raise_resume.dynasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=431 +guard_failures=482 internal_compile_panics=0 loops_aborted=1 loops_compiled=36 diff --git a/pyre/bench/synth/sre_pattern_methods.cranelift.jitstats b/pyre/bench/synth/sre_pattern_methods.cranelift.jitstats index 8c36dc55db3..b549d05062f 100644 --- a/pyre/bench/synth/sre_pattern_methods.cranelift.jitstats +++ b/pyre/bench/synth/sre_pattern_methods.cranelift.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=8 +bridges_compiled=11 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1670 +guard_failures=2291 internal_compile_panics=0 loops_aborted=0 loops_compiled=7 diff --git a/pyre/bench/synth/sre_pattern_methods.dynasm.jitstats b/pyre/bench/synth/sre_pattern_methods.dynasm.jitstats index 8c36dc55db3..b549d05062f 100644 --- a/pyre/bench/synth/sre_pattern_methods.dynasm.jitstats +++ b/pyre/bench/synth/sre_pattern_methods.dynasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=8 +bridges_compiled=11 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1670 +guard_failures=2291 internal_compile_panics=0 loops_aborted=0 loops_compiled=7 diff --git a/pyre/bench/synth/sre_wasm_min.cranelift.jitstats b/pyre/bench/synth/sre_wasm_min.cranelift.jitstats index 4b40fe29a17..00169699651 100644 --- a/pyre/bench/synth/sre_wasm_min.cranelift.jitstats +++ b/pyre/bench/synth/sre_wasm_min.cranelift.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=5 +bridges_compiled=8 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1161 +guard_failures=1849 internal_compile_panics=0 loops_aborted=0 loops_compiled=5 diff --git a/pyre/bench/synth/sre_wasm_min.dynasm.jitstats b/pyre/bench/synth/sre_wasm_min.dynasm.jitstats index 4b40fe29a17..00169699651 100644 --- a/pyre/bench/synth/sre_wasm_min.dynasm.jitstats +++ b/pyre/bench/synth/sre_wasm_min.dynasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=5 +bridges_compiled=8 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1161 +guard_failures=1849 internal_compile_panics=0 loops_aborted=0 loops_compiled=5 diff --git a/pyre/bench/synth/sre_wasm_min1.cranelift.jitstats b/pyre/bench/synth/sre_wasm_min1.cranelift.jitstats index 27b0f897e10..26e4d7b2337 100644 --- a/pyre/bench/synth/sre_wasm_min1.cranelift.jitstats +++ b/pyre/bench/synth/sre_wasm_min1.cranelift.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=3 +bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=603 +guard_failures=803 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/sre_wasm_min1.dynasm.jitstats b/pyre/bench/synth/sre_wasm_min1.dynasm.jitstats index 27b0f897e10..26e4d7b2337 100644 --- a/pyre/bench/synth/sre_wasm_min1.dynasm.jitstats +++ b/pyre/bench/synth/sre_wasm_min1.dynasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=3 +bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=603 +guard_failures=803 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/str_index_bytes_iter_surface.cranelift.jitstats b/pyre/bench/synth/str_index_bytes_iter_surface.cranelift.jitstats index 14cb3c66bf6..f6a9f28ffaf 100644 --- a/pyre/bench/synth/str_index_bytes_iter_surface.cranelift.jitstats +++ b/pyre/bench/synth/str_index_bytes_iter_surface.cranelift.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=461 +guard_failures=640 internal_compile_panics=0 loops_aborted=0 loops_compiled=10 diff --git a/pyre/bench/synth/str_index_bytes_iter_surface.dynasm.jitstats b/pyre/bench/synth/str_index_bytes_iter_surface.dynasm.jitstats index 14cb3c66bf6..f6a9f28ffaf 100644 --- a/pyre/bench/synth/str_index_bytes_iter_surface.dynasm.jitstats +++ b/pyre/bench/synth/str_index_bytes_iter_surface.dynasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=461 +guard_failures=640 internal_compile_panics=0 loops_aborted=0 loops_compiled=10 diff --git a/pyre/extra_tests/parity_tests/list_object_append_fold.py b/pyre/extra_tests/parity_tests/list_object_append_fold.py new file mode 100644 index 00000000000..4163b6b8a51 --- /dev/null +++ b/pyre/extra_tests/parity_tests/list_object_append_fold.py @@ -0,0 +1,201 @@ +# Object-strategy `list.append`, run hot enough to be traced. +# +# The append fold descends the real `w_list_append` body. Its Object arm +# stores a GC ref, so unlike the unboxed Integer/Float arms it first runs the +# list write barrier through `prepare_list_ref_store`, which returns the value +# at its post-safepoint address. Getting that wrong stores a stale pointer, so +# every case below reads back what it appended and folds it into a checksum. +# +# The fold only fires while the backing block has spare capacity; a full block +# side-exits to the resizing push. Each loop therefore crosses several +# reallocation boundaries, and the interleaved `pop` / `insert` / slot +# assignment keep exercising the same block from the non-folded side. + +N = 400 + + +def check(got, want, label): + assert got == want, "%s: %r != %r" % (label, got, want) + + +# ── grow from empty, read every element back ── +def append_str(n): + xs = [] + i = 0 + while i < n: + xs.append("e%d" % (i % 7)) + i = i + 1 + total = 0 + for s in xs: + total = total + int(s[1:]) + return total + + +def append_tuple(n): + xs = [] + i = 0 + while i < n: + xs.append((i, i + 1)) + i = i + 1 + total = 0 + for a, b in xs: + total = total + b - a + return total + + +def append_none(n): + xs = [] + i = 0 + while i < n: + xs.append(None) + i = i + 1 + return len(xs) + sum(1 for x in xs if x is None) + + +# ── a sliding window: the block never grows past the bound, so the fold's +# spare-capacity guard holds while `pop(0)` keeps rewriting the same slots ── +def append_pop_window(n): + xs = [] + marker = "m" + i = 0 + acc = 0 + while i < n: + xs.append(marker) + if len(xs) > 32: + acc = acc + len(xs.pop(0)) + i = i + 1 + return acc * 1000 + len(xs) + + +# ── a heterogeneous list: the Object strategy is the only one that can hold +# it, and the appended element's type changes every iteration ── +def append_mixed(n): + xs = [] + i = 0 + while i < n: + r = i % 4 + if r == 0: + xs.append(i) + elif r == 1: + xs.append(str(i)) + elif r == 2: + xs.append(None) + else: + xs.append((i,)) + i = i + 1 + ints = 0 + strs = 0 + nones = 0 + tups = 0 + for x in xs: + if x is None: + nones = nones + 1 + elif isinstance(x, tuple): + tups = tups + x[0] % 2 + elif isinstance(x, str): + strs = strs + len(x) + else: + ints = ints + 1 + return ints * 1000000 + strs * 10000 + nones * 100 + tups + + +# ── an int list that de-specialises mid-loop: the first non-int append +# switches the storage to Object, so the same call site folds through two +# different arms ── +def append_despecialise(n): + xs = [] + i = 0 + while i < n: + if i == n // 2: + xs.append("wall") + else: + xs.append(i) + i = i + 1 + total = 0 + for x in xs: + if isinstance(x, str): + total = total + len(x) * 100000 + else: + total = total + x + return total + + +# ── a store into an already-appended slot, so the barrier runs on both the +# append and the assignment ── +def append_then_store(n): + xs = [] + i = 0 + while i < n: + xs.append("a") + xs[len(xs) - 1] = "b%d" % (i % 5) + i = i + 1 + total = 0 + for s in xs: + total = total + int(s[1:]) + return total + + +# ── an insert in the middle keeps the block shifting under the fold ── +def append_and_insert(n): + xs = [] + i = 0 + while i < n: + xs.append("t") + if i % 16 == 0: + xs.insert(len(xs) // 2, "i") + i = i + 1 + return len(xs) * 100 + sum(1 for s in xs if s == "i") + + +# ── two lists alternating at one call site: a receiver mix-up cross-checks ── +def append_two_receivers(n): + a = [] + b = [] + i = 0 + while i < n: + target = a if i % 2 == 0 else b + target.append("x%d" % i) + i = i + 1 + return len(a) * 1000 + len(b) + int(a[0][1:]) + int(b[0][1:]) + + +def run(): + check(append_str(N), sum(i % 7 for i in range(N)), "append_str") + check(append_tuple(N), N, "append_tuple") + check(append_none(N), 2 * N, "append_none") + check(append_pop_window(N), (N - 32) * 1000 + 32, "append_pop_window") + check(append_mixed(N), expected_mixed(), "append_mixed") + check(append_despecialise(N), sum(range(N)) - N // 2 + 400000, "append_despecialise") + check(append_then_store(N), sum(i % 5 for i in range(N)), "append_then_store") + check(append_and_insert(N), (N + 25) * 100 + 25, "append_and_insert") + check(append_two_receivers(N), 200 * 1000 + 200 + 0 + 1, "append_two_receivers") + + +# `append_mixed`'s checksum is written out rather than recomputed so a wrong +# element cannot be cancelled by a matching wrong expectation. +def expected_mixed(): + ints = 0 + strs = 0 + nones = 0 + tups = 0 + for i in range(N): + r = i % 4 + if r == 0: + ints = ints + 1 + elif r == 1: + strs = strs + len(str(i)) + elif r == 2: + nones = nones + 1 + else: + tups = tups + i % 2 + return ints * 1000000 + strs * 10000 + nones * 100 + tups + + +check(append_mixed(N), expected_mixed(), "append_mixed") +run() + +# A second pass on already-warm code: every loop above has been traced by now, +# so this run executes the compiled form rather than building it. +run() + +print("OK") diff --git a/pyre/pyre-interpreter/src/jit_fnaddr.rs b/pyre/pyre-interpreter/src/jit_fnaddr.rs index 7c108f813ff..114eeed4dee 100644 --- a/pyre/pyre-interpreter/src/jit_fnaddr.rs +++ b/pyre/pyre-interpreter/src/jit_fnaddr.rs @@ -284,7 +284,12 @@ pub fn is_pyframe_operand_stack_accessor(addr: usize) -> bool { addrs.contains(&(addr as i64)) } -/// True when `addr` is the `list_write_barrier` residual fnaddr. +/// True when `addr` is the list write barrier's residual fnaddr, in either the +/// bare [`pyre_object::list_write_barrier`] spelling or the +/// `prepare_list_ref_store` wrapper the Object-strategy store goes through +/// (the wrapper only adds the `push_roots` bracket that keeps the stored value +/// addressable across the barrier's safepoint — the same bookkeeping, so the +/// same exemption). /// /// The #171 object-append fold descends `w_list_append`; its Object-strategy /// arm stores a GC ref and runs `list_write_barrier(obj)` @@ -318,6 +323,8 @@ pub fn is_list_write_barrier(addr: usize) -> bool { .filter(|(path, _)| { path.ends_with("::listobject::list_write_barrier") || *path == "pyre_object::list_write_barrier" + || path.ends_with("::listobject::prepare_list_ref_store") + || *path == "pyre_object::prepare_list_ref_store" }) .map(|(_, fnaddr)| fnaddr) .collect() @@ -1713,6 +1720,23 @@ pub fn jit_trace_fnaddrs() -> Vec<(&'static str, i64)> { "pyre_object::list_write_barrier", pyre_object::list_write_barrier as *const (), ); + // The Object arm reaches the barrier through `prepare_list_ref_store`, + // which brackets it in `push_roots` so the value survives the safepoint + // inside the barrier's ownership query. That bracket's zero-arg + // root-stack resolve has no registered address, so leaving it inside the + // descended body made every object-strategy append decline the fold. The + // wrapper is `dont_look_inside`; register it for the same reason as the + // barrier itself. + let prepare_list_ref_store: fn( + pyre_object::PyObjectRef, + pyre_object::PyObjectRef, + ) -> pyre_object::PyObjectRef = pyre_object::listobject::prepare_list_ref_store; + push_alias_pair( + &mut entries, + "pyre_object::listobject::prepare_list_ref_store", + "pyre_object::prepare_list_ref_store", + prepare_list_ref_store as *const (), + ); // The #171 fold descends `w_list_append` as a sub-jitcode walk, so a guard // exit inside it is numbered against `w_list_append`'s own jitcode and is // resumed there in the blackhole (`resume.py:1339 jitcodes[jitcode_pos]`). diff --git a/pyre/pyre-object/src/listobject.rs b/pyre/pyre-object/src/listobject.rs index 661a180470a..3fd94ad58ee 100644 --- a/pyre/pyre-object/src/listobject.rs +++ b/pyre/pyre-object/src/listobject.rs @@ -638,7 +638,22 @@ fn list_write_barrier_impl(obj: PyObjectRef, managed: bool) { /// shadow-stack slot. A concurrent collector may run while the barrier waits /// for the GC operation gate, so the raw Rust argument must be reloaded before /// the following pointer store. -fn prepare_list_ref_store(obj: PyObjectRef, value: PyObjectRef) -> PyObjectRef { +/// +/// `dont_look_inside`: the `push_roots` bracket around the barrier resolves the +/// thread-local root stack and installs a `Drop` that truncates it — a +/// shadow-stack shape with no RPython counterpart (the GC transform emits +/// `ll_writebarrier` with no bracket at all) and no lowering in the tracer, so +/// the object-append descent used to hit its unregistered zero-arg callee as a +/// `symbolic_fnaddr_for_path` hash and decline the whole fold. Collapsing the +/// bracket and the barrier into one registered residual keeps the Object arm's +/// `set_len` / `setitem_fast` leaves foldable to native ops, and costs the same +/// one residual call the barrier alone already did. +/// +/// Returns `value` at its post-barrier address: the ownership query inside +/// `list_write_barrier` is a safepoint, so the caller must store the returned +/// pointer rather than the argument it passed. +#[majit_macros::dont_look_inside] +pub fn prepare_list_ref_store(obj: PyObjectRef, value: PyObjectRef) -> PyObjectRef { let _roots = crate::gc_roots::push_roots(); let value_slot = crate::gc_roots::shadow_stack_len(); crate::gc_roots::pin_root(value); From fd45a4f0bc56873f797817fdacb197ec48f605a3 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 5 Aug 2026 20:18:12 +0900 Subject: [PATCH 04/10] wasm: size the value space from every operand, and give the append fold's residual call the uniform word ABI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects kept the object-strategy `list.append` fold from being correct on the wasm backend. `collect_guards_and_vars` raised `max_var` for input args, op results and `LABEL` args only, never for an ordinary op argument. A value the constants pool alone binds — no producing op, the case `unbound_pool_const_seeds` exists to seed in the prologue — therefore did not raise it. `next_value_pos` is that same count, and `majit_gc::rewrite::remove_ref_constants` numbers the `LoadFromGcTable` results it inserts from there upward, so the load took the pool-bound value's id. The two then shared one local, and because the value now had a producing op `unbound_pool_const_seeds` emitted no prologue store — its own `raw >= num_vars` guard had been skipping the value as well. The read at the earlier op preceded the store. Concretely a `NewArrayClear` length read the zero wasm initializes a local to, so the allocated `ItemsBlock` had capacity 0 while the trace set `length = 1` and stored `items[0]`. The next interpreted `append` saw `length != capacity`, skipped the grow and wrote past the block; the next allocation's type-id header then overwrote that word. `sre_wasm_min` answered 29818 for 30000 and `exception_traceback_lineno_chain` mismatched. A residual call target must carry the uniform word ABI: the backend lowers an `Int`/`Ref`-result residual to a `call_indirect` whose static type comes from the descr alone, and a raw `(*mut PyObject, *mut PyObject) -> *mut PyObject` is `(i32, i32) -> i32` on wasm32, which traps `indirect call type mismatch`. Spell `prepare_list_ref_store`'s signature with `*mut PyObject` so `emit_helper_call_target_fn` emits the `extern "C" fn(i64, i64) -> i64` trampoline, and register that trampoline instead of the raw fn. The registered paths are unchanged, so `is_list_write_barrier` and the path-keyed build-to-runtime re-pairing are unaffected. 14 wasm jit-stats patterns re-recorded — the append fold's deltas, a strict subset of the 16 the native backends record (`exception_catching_frame_tb_node` and `exception_traceback_lineno_chain` move on the natives only). check.py: dynasm 382/382, cranelift 382/382, wasm 378/378. Assisted-by: Claude --- majit/majit-backend-wasm/src/codegen.rs | 31 ++++++++++++++----- ...prehension_object_append_hot.wasm.jitstats | 4 +-- .../synth/const_arg_call_resume.wasm.jitstats | 4 +-- ...tion_inline_callee_tb_frames.wasm.jitstats | 2 +- ...entry_guard_finally_residual.wasm.jitstats | 2 +- ...ption_traceback_frame_lineno.wasm.jitstats | 2 +- ...ridge_flavor_traceback_names.wasm.jitstats | 2 +- ...list_append_write_barrier_gc.wasm.jitstats | 2 +- .../synth/minmax_key_rooting.wasm.jitstats | 2 +- ...ested_list_comprehension_hot.wasm.jitstats | 4 +-- ...pickle_terminal_raise_resume.wasm.jitstats | 2 +- .../synth/sre_pattern_methods.wasm.jitstats | 4 +-- pyre/bench/synth/sre_wasm_min.wasm.jitstats | 4 +-- pyre/bench/synth/sre_wasm_min1.wasm.jitstats | 4 +-- ...str_index_bytes_iter_surface.wasm.jitstats | 2 +- pyre/pyre-interpreter/src/jit_fnaddr.rs | 19 +++++++++--- pyre/pyre-object/src/listobject.rs | 11 ++++++- 17 files changed, 68 insertions(+), 33 deletions(-) diff --git a/majit/majit-backend-wasm/src/codegen.rs b/majit/majit-backend-wasm/src/codegen.rs index 950fbeee84e..cd5ca144943 100644 --- a/majit/majit-backend-wasm/src/codegen.rs +++ b/majit/majit-backend-wasm/src/codegen.rs @@ -669,8 +669,10 @@ pub fn label_ref_capture_slots(inputargs: &[InputArg], ops: &[Op]) -> usize { LabelResumeData::collect(inputargs, ops).ref_slots } -/// First free value position — one past the highest id any input arg or op -/// result occupies. `majit_gc::rewrite::remove_ref_constants` numbers the +/// First free value position — one past the highest id any value reference in +/// the trace occupies (input args, op results, and every op argument, including +/// a folded value the constants pool alone binds). +/// `majit_gc::rewrite::remove_ref_constants` numbers the /// `LoadFromGcTable` results it emits from here upward, so the operand /// numbering the optimizer produced stays untouched. Same id set /// `collect_guards_and_vars` sizes `num_vars` from, so the loads land inside @@ -1303,12 +1305,25 @@ fn collect_guards_and_vars(inputargs: &[InputArg], ops: &[Op]) -> (Vec max_var { - max_var = a.raw() + 1; - } + // Every value an op reads occupies a local, whether or not the trace + // also contains an op that produces it: constant folding and the short + // preamble leave a folded value bound only by the constants pool, and + // `unbound_pool_const_seeds` materializes it in the prologue. Counting + // only op results would under-size `num_vars` for such a value and, + // through `next_value_pos`, let `remove_ref_constants` reuse its id for + // a `LoadFromGcTable` — whose store then lands after the read, so the + // read returns the zero wasm initializes the local to. + let mut widen = |a: OpRef, max_var: &mut u32| { + if a != OpRef::NONE && !a.is_constant() && a.raw() + 1 > *max_var { + *max_var = a.raw() + 1; + } + }; + for a in op.getarglist().iter() { + widen(a.to_opref(), &mut max_var); + } + if let Some(fa) = op.getfailargs() { + for a in fa.iter() { + widen(a.to_opref(), &mut max_var); } } diff --git a/pyre/bench/synth/comprehension_object_append_hot.wasm.jitstats b/pyre/bench/synth/comprehension_object_append_hot.wasm.jitstats index b845ac1b8ba..162430619de 100644 --- a/pyre/bench/synth/comprehension_object_append_hot.wasm.jitstats +++ b/pyre/bench/synth/comprehension_object_append_hot.wasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=8 +bridges_compiled=18 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1605 +guard_failures=3610 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/const_arg_call_resume.wasm.jitstats b/pyre/bench/synth/const_arg_call_resume.wasm.jitstats index 82f9d708660..f7e39b7e145 100644 --- a/pyre/bench/synth/const_arg_call_resume.wasm.jitstats +++ b/pyre/bench/synth/const_arg_call_resume.wasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=7 +bridges_compiled=9 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1404 +guard_failures=1804 internal_compile_panics=0 loops_aborted=0 loops_compiled=3 diff --git a/pyre/bench/synth/exception_inline_callee_tb_frames.wasm.jitstats b/pyre/bench/synth/exception_inline_callee_tb_frames.wasm.jitstats index b9919343891..cfdee3cf187 100644 --- a/pyre/bench/synth/exception_inline_callee_tb_frames.wasm.jitstats +++ b/pyre/bench/synth/exception_inline_callee_tb_frames.wasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=608 +guard_failures=1685 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/exception_reentry_guard_finally_residual.wasm.jitstats b/pyre/bench/synth/exception_reentry_guard_finally_residual.wasm.jitstats index cc13e71d123..a43dc131d42 100644 --- a/pyre/bench/synth/exception_reentry_guard_finally_residual.wasm.jitstats +++ b/pyre/bench/synth/exception_reentry_guard_finally_residual.wasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=2261 +guard_failures=2461 internal_compile_panics=0 loops_aborted=0 loops_compiled=3 diff --git a/pyre/bench/synth/exception_traceback_frame_lineno.wasm.jitstats b/pyre/bench/synth/exception_traceback_frame_lineno.wasm.jitstats index 9161a11c628..eff716cf5be 100644 --- a/pyre/bench/synth/exception_traceback_frame_lineno.wasm.jitstats +++ b/pyre/bench/synth/exception_traceback_frame_lineno.wasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=818 +guard_failures=815 internal_compile_panics=0 loops_aborted=0 loops_compiled=17 diff --git a/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.wasm.jitstats b/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.wasm.jitstats index 5b7442125b5..c9c01e5d40b 100644 --- a/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.wasm.jitstats +++ b/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.wasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1836 +guard_failures=2036 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats b/pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats index 84a1bea3f6a..478af581564 100644 --- a/pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats +++ b/pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1335 +guard_failures=1345 internal_compile_panics=0 loops_aborted=1 loops_compiled=12 diff --git a/pyre/bench/synth/minmax_key_rooting.wasm.jitstats b/pyre/bench/synth/minmax_key_rooting.wasm.jitstats index 8beed56f050..ce08616f8db 100644 --- a/pyre/bench/synth/minmax_key_rooting.wasm.jitstats +++ b/pyre/bench/synth/minmax_key_rooting.wasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1 +guard_failures=5 internal_compile_panics=0 loops_aborted=0 loops_compiled=1 diff --git a/pyre/bench/synth/nested_list_comprehension_hot.wasm.jitstats b/pyre/bench/synth/nested_list_comprehension_hot.wasm.jitstats index f4509ffc3bb..c98bf212154 100644 --- a/pyre/bench/synth/nested_list_comprehension_hot.wasm.jitstats +++ b/pyre/bench/synth/nested_list_comprehension_hot.wasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=2 +bridges_compiled=6 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=401 +guard_failures=1202 internal_compile_panics=0 loops_aborted=0 loops_compiled=2 diff --git a/pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats b/pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats index 6e824e9a554..e6d2a5d24d9 100644 --- a/pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats +++ b/pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=5 -guard_failures=432 +guard_failures=483 internal_compile_panics=0 loops_aborted=13 loops_compiled=73 diff --git a/pyre/bench/synth/sre_pattern_methods.wasm.jitstats b/pyre/bench/synth/sre_pattern_methods.wasm.jitstats index 1a60015329e..e526333d4a8 100644 --- a/pyre/bench/synth/sre_pattern_methods.wasm.jitstats +++ b/pyre/bench/synth/sre_pattern_methods.wasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=8 +bridges_compiled=11 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1671 +guard_failures=2292 internal_compile_panics=0 loops_aborted=0 loops_compiled=7 diff --git a/pyre/bench/synth/sre_wasm_min.wasm.jitstats b/pyre/bench/synth/sre_wasm_min.wasm.jitstats index 4b40fe29a17..00169699651 100644 --- a/pyre/bench/synth/sre_wasm_min.wasm.jitstats +++ b/pyre/bench/synth/sre_wasm_min.wasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=5 +bridges_compiled=8 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1161 +guard_failures=1849 internal_compile_panics=0 loops_aborted=0 loops_compiled=5 diff --git a/pyre/bench/synth/sre_wasm_min1.wasm.jitstats b/pyre/bench/synth/sre_wasm_min1.wasm.jitstats index 27b0f897e10..26e4d7b2337 100644 --- a/pyre/bench/synth/sre_wasm_min1.wasm.jitstats +++ b/pyre/bench/synth/sre_wasm_min1.wasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=3 +bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=603 +guard_failures=803 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/str_index_bytes_iter_surface.wasm.jitstats b/pyre/bench/synth/str_index_bytes_iter_surface.wasm.jitstats index 14cb3c66bf6..f6a9f28ffaf 100644 --- a/pyre/bench/synth/str_index_bytes_iter_surface.wasm.jitstats +++ b/pyre/bench/synth/str_index_bytes_iter_surface.wasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=461 +guard_failures=640 internal_compile_panics=0 loops_aborted=0 loops_compiled=10 diff --git a/pyre/pyre-interpreter/src/jit_fnaddr.rs b/pyre/pyre-interpreter/src/jit_fnaddr.rs index 114eeed4dee..2f77d089b71 100644 --- a/pyre/pyre-interpreter/src/jit_fnaddr.rs +++ b/pyre/pyre-interpreter/src/jit_fnaddr.rs @@ -1727,10 +1727,21 @@ pub fn jit_trace_fnaddrs() -> Vec<(&'static str, i64)> { // descended body made every object-strategy append decline the fold. The // wrapper is `dont_look_inside`; register it for the same reason as the // barrier itself. - let prepare_list_ref_store: fn( - pyre_object::PyObjectRef, - pyre_object::PyObjectRef, - ) -> pyre_object::PyObjectRef = pyre_object::listobject::prepare_list_ref_store; + // + // Register the macro-emitted `extern "C" fn(i64, i64) -> i64` call + // trampoline, not the raw fn — the shape + // `#[jit_module]::__majit_helper_trace_fnaddrs()` publishes for a + // policy-bearing free fn (`majit-macros` `impl_addr_expr` routes it through + // `__majit_call_policy_*`'s trace-target slot; the raw fn is only its + // null-target fallback). The wasm backend lowers an `Int`/`Ref`-result + // residual to a direct `call_indirect` whose static type is `(i64 x n) -> + // i64` derived from the descr alone, so a raw + // `(*mut PyObject, *mut PyObject) -> *mut PyObject` — `(i32, i32) -> i32` on + // wasm32 — traps `indirect call type mismatch`. The registered paths are + // unchanged, so `is_list_write_barrier` and the path-keyed build->runtime + // re-pairing are unaffected. + let prepare_list_ref_store: extern "C" fn(i64, i64) -> i64 = + pyre_object::listobject::__majit_call_target_prepare_list_ref_store; push_alias_pair( &mut entries, "pyre_object::listobject::prepare_list_ref_store", diff --git a/pyre/pyre-object/src/listobject.rs b/pyre/pyre-object/src/listobject.rs index 3fd94ad58ee..f1b23a925cc 100644 --- a/pyre/pyre-object/src/listobject.rs +++ b/pyre/pyre-object/src/listobject.rs @@ -652,8 +652,17 @@ fn list_write_barrier_impl(obj: PyObjectRef, managed: bool) { /// Returns `value` at its post-barrier address: the ownership query inside /// `list_write_barrier` is a safepoint, so the caller must store the returned /// pointer rather than the argument it passed. +/// +/// The signature spells `*mut PyObject` rather than the identical `PyObjectRef` +/// alias so that `emit_helper_call_target_fn` recognises the parameters and the +/// result as raw pointers and emits the `extern "C" fn(i64, i64) -> i64` call +/// trampoline. `jit_fnaddr.rs` registers that trampoline: a residual call's +/// target must carry the uniform word ABI, because the wasm backend lowers an +/// `Int`/`Ref`-result residual to a `call_indirect` whose static type comes from +/// the descr alone, and a raw `(*mut PyObject, *mut PyObject) -> *mut PyObject` +/// is `(i32, i32) -> i32` on wasm32. #[majit_macros::dont_look_inside] -pub fn prepare_list_ref_store(obj: PyObjectRef, value: PyObjectRef) -> PyObjectRef { +pub fn prepare_list_ref_store(obj: *mut PyObject, value: *mut PyObject) -> *mut PyObject { let _roots = crate::gc_roots::push_roots(); let value_slot = crate::gc_roots::shadow_stack_len(); crate::gc_roots::pin_root(value); From ac77a3b6bb60e9a64e9ea56290c30c4cd28dc3c8 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 5 Aug 2026 23:06:22 +0900 Subject: [PATCH 05/10] dict: make the strategy slot one word, so it can be guarded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `W_DictObject.dstrategy` held a `&'static dyn DictStrategy`. Upstream's `dstrategy` (`dictmultiobject.py:325`) is a single instance pointer, and `W_ModuleDictObject.mstrategy` beside it already is one (`*mut ModuleDictStrategy`); the regular dict was the outlier. The deviation costs two things. A Rust `&dyn` carries the vtable in the *reference*, so the dict has no field holding the strategy's identity for a `guard_class` to read — which is how RPython pins the receiver of the virtual `get_strategy().getitem(..)` call it inlines, and therefore the shape any traced dict lookup needs. And the singletons are unit structs, so the reference's data word is the address of a zero-sized static, which Rust does not guarantee to differ between distinct strategies. The trait's own doc records the consequence — `strategy_kind()` exists "because pointer comparison on the `&'static dyn DictStrategy` slot is unreliable for ZST strategies" — and `W_ModuleDictObject::set_strategy` was performing exactly that unreliable comparison, casting both sides to `*const ()` to test for `OBJECT_DICT_STRATEGY`. Introduce `DictStrategyRef`, a `#[repr(C)]` holder for the trait object, and one `static` per singleton. `dstrategy` becomes `&'static DictStrategyRef`: one word, and distinct per strategy because the holders are not zero-sized. `DictStrategyRef` derefs to `dyn DictStrategy`, so every dispatch through the slot is unchanged. `set_strategy` and the two allocator entry points take the holder; `get_strategy` still hands out the trait object. The module dict's identity test becomes a `ptr::eq` on holders. `DictStrategy` cannot carry a `Sync` bound — `ModuleDictStrategy`'s `GlobalCache` holds `*mut PyObject` — so the holder asserts it instead; a module dict never uses one. cargo test --all --no-default-features --features dynasm: 7445 passed, 0 failed, 101 targets. Assisted-by: Claude --- .../src/objspace/std/mapdict.rs | 19 +- pyre/pyre-object/src/dictmultiobject.rs | 172 ++++++++++++------ pyre/pyre-object/src/identitydict.rs | 13 +- pyre/pyre-object/src/kwargsdict.rs | 15 +- 4 files changed, 149 insertions(+), 70 deletions(-) diff --git a/pyre/pyre-interpreter/src/objspace/std/mapdict.rs b/pyre/pyre-interpreter/src/objspace/std/mapdict.rs index 4a7a75edd72..bc79795c677 100644 --- a/pyre/pyre-interpreter/src/objspace/std/mapdict.rs +++ b/pyre/pyre-interpreter/src/objspace/std/mapdict.rs @@ -3721,7 +3721,7 @@ pub unsafe fn mapdict_switch_to_object_strategy(w_dict: PyObjectRef) { // w_dict.dstorage = strategy.erase(dict_w). let dict = unsafe { &mut *(w_dict as *mut pyre_object::W_DictObject) }; dict.dstorage = pyre_object::dictmultiobject::OBJECT_DICT_STRATEGY.get_empty_storage(); - dict.dstrategy = &pyre_object::dictmultiobject::OBJECT_DICT_STRATEGY; + dict.dstrategy = &pyre_object::dictmultiobject::OBJECT_DICT_STRATEGY_REF; // materialize_r_dict(space, w_obj, dict_w). unsafe { materialize_dict(w_obj, w_dict) }; } @@ -3739,7 +3739,7 @@ pub unsafe fn mapdict_switch_to_text_strategy(w_dict: PyObjectRef) { let w_obj = unsafe { mapdict_strategy_unerase(w_dict) }; let dict = unsafe { &mut *(w_dict as *mut pyre_object::W_DictObject) }; dict.dstorage = pyre_object::dictmultiobject::UNICODE_DICT_STRATEGY.get_empty_storage(); - dict.dstrategy = &pyre_object::dictmultiobject::UNICODE_DICT_STRATEGY; + dict.dstrategy = &pyre_object::dictmultiobject::UNICODE_DICT_STRATEGY_REF; // materialize_str_dict(space, w_obj, str_dict). unsafe { materialize_dict(w_obj, w_dict) }; } @@ -3755,6 +3755,13 @@ pub struct MapDictStrategy; /// ZST contract as [`pyre_object::dictmultiobject::OBJECT_DICT_STRATEGY`]. pub static MAP_DICT_STRATEGY: MapDictStrategy = MapDictStrategy; +/// The [`pyre_object::dictmultiobject::DictStrategyRef`] holder a dict's +/// `dstrategy` slot points at. +pub static MAP_DICT_STRATEGY_REF: pyre_object::dictmultiobject::DictStrategyRef = + pyre_object::dictmultiobject::DictStrategyRef { + imp: &MAP_DICT_STRATEGY, + }; + impl pyre_object::dictmultiobject::DictStrategy for MapDictStrategy { fn strategy_kind(&self) -> pyre_object::dictmultiobject::StrategyKind { pyre_object::dictmultiobject::StrategyKind::Map @@ -3956,7 +3963,7 @@ pub fn _obj_getdict(self_ref: PyObjectRef) -> PyObjectRef { let self_slot = pyre_object::gc_roots::pin_roots(&[self_ref]); let dict_slot = self_slot + 1; pyre_object::gc_roots::pin_root(pyre_object::w_dict_new_with( - &MAP_DICT_STRATEGY, + &MAP_DICT_STRATEGY_REF, pyre_object::gc_roots::shadow_stack_get(self_slot) as *mut u8, )); unsafe { @@ -4885,7 +4892,7 @@ mod tests { assert!(instance_node_setdictvalue(obj_ref, wn("x"), sentinel(0x11))); assert!(instance_node_setdictvalue(obj_ref, wn("y"), sentinel(0x22))); - let w_dict = pyre_object::w_dict_new_with(&MAP_DICT_STRATEGY, obj_ref as *mut u8); + let w_dict = pyre_object::w_dict_new_with(&MAP_DICT_STRATEGY_REF, obj_ref as *mut u8); assert_eq!(MAP_DICT_STRATEGY.strategy_kind(), StrategyKind::Map); assert_eq!(MAP_DICT_STRATEGY.length(w_dict), 2); @@ -4947,7 +4954,7 @@ mod tests { assert!(instance_node_setdictvalue(obj_ref, wn("x"), sentinel(0x11))); assert!(instance_node_setdictvalue(obj_ref, wn("y"), sentinel(0x22))); - let w_dict = pyre_object::w_dict_new_with(&MAP_DICT_STRATEGY, obj_ref as *mut u8); + let w_dict = pyre_object::w_dict_new_with(&MAP_DICT_STRATEGY_REF, obj_ref as *mut u8); assert_eq!(MAP_DICT_STRATEGY.length(w_dict), 2); // A non-str key forces switch_to_object_strategy → materialise. @@ -4988,7 +4995,7 @@ mod tests { obj._set_mapdict_map(term); assert!(instance_node_setdictvalue(obj_ref, wn("a"), sentinel(0x55))); - let w_dict = pyre_object::w_dict_new_with(&MAP_DICT_STRATEGY, obj_ref as *mut u8); + let w_dict = pyre_object::w_dict_new_with(&MAP_DICT_STRATEGY_REF, obj_ref as *mut u8); // The LIMIT-devolve path (mapdict.py:317-323) switches to text. mapdict_switch_to_text_strategy(w_dict); diff --git a/pyre/pyre-object/src/dictmultiobject.rs b/pyre/pyre-object/src/dictmultiobject.rs index d202993dad1..e48299e8aea 100644 --- a/pyre/pyre-object/src/dictmultiobject.rs +++ b/pyre/pyre-object/src/dictmultiobject.rs @@ -746,11 +746,11 @@ pub trait W_DictMultiObject { /// `dictmultiobject.py:52-53 W_DictMultiObject.set_strategy` /// abstract method, overridden by `W_DictObject` (`:324-325`) and /// `W_ModuleDictObject` (`:341-342`). Pyre limits the setter to - /// `&'static dyn DictStrategy` (the singleton dispatch surface); + /// a `&'static DictStrategyRef` (the singleton dispatch surface); /// W_ModuleDictObject strategy promotion to ObjectDictStrategy /// continues to go through `w_module_dict_switch_to_object_strategy` /// per `celldict.py:173-186`. - fn set_strategy(&mut self, strategy: &'static dyn crate::dictmultiobject::DictStrategy); + fn set_strategy(&mut self, strategy: &'static DictStrategyRef); } /// `pypy/objspace/std/dictmultiobject.py:313-325 W_DictObject(W_DictMultiObject)` @@ -789,7 +789,17 @@ pub struct W_DictObject { /// holder ([`w_dict_new_unmanaged_side_table_value`]) keeps an off-GC /// `malloc_raw` box. pub dstorage: *mut u8, - pub dstrategy: &'static dyn crate::dictmultiobject::DictStrategy, + /// `dstrategy` from `W_DictObject.__slots__` (`dictmultiobject.py:325`) — + /// **one word**, like `W_ModuleDictObject.mstrategy` beside it and like the + /// single instance pointer upstream stores. The strategy singletons are + /// unit structs, so a `&'static dyn DictStrategy` here would be a fat + /// pointer carrying the vtable in the *reference*: no field for the JIT to + /// read, and a data word that Rust may collapse to one address across + /// distinct ZST statics. [`DictStrategyRef`] holds the trait object + /// instead and derefs to it, so this slot is a plain pointer that a + /// `guard_class` can pin the way RPython pins an inlined virtual call's + /// receiver. + pub dstrategy: &'static DictStrategyRef, /// Mutation state carried implicitly by PyPy's live strategy iterator. /// `IndexMap::shift_remove` compacts its storage, so pyre records key-set /// changes explicitly; value-only overwrites leave this unchanged. @@ -1006,11 +1016,11 @@ impl crate::lltype::GcType for W_DictObject { impl W_DictMultiObject for W_DictObject { #[inline] fn get_strategy(&self) -> &dyn crate::dictmultiobject::DictStrategy { - self.dstrategy + self.dstrategy.imp } #[inline] - fn set_strategy(&mut self, strategy: &'static dyn crate::dictmultiobject::DictStrategy) { + fn set_strategy(&mut self, strategy: &'static DictStrategyRef) { self.dstrategy = strategy; } } @@ -1047,7 +1057,7 @@ pub unsafe fn w_dict_get_strategy( return &*strat_ptr; } let dict = &*(obj as *const W_DictObject); - dict.dstrategy + dict.dstrategy.imp } /// `pypy/objspace/std/dictmultiobject.py:52-53 W_DictMultiObject.set_strategy` @@ -1067,10 +1077,7 @@ pub unsafe fn w_dict_get_strategy( /// `obj` must be a valid PyObjectRef pointing at a `W_DictObject` or /// `W_ModuleDictObject`. #[inline] -pub unsafe fn w_dict_set_strategy( - obj: PyObjectRef, - strategy: &'static dyn crate::dictmultiobject::DictStrategy, -) { +pub unsafe fn w_dict_set_strategy(obj: PyObjectRef, strategy: &'static DictStrategyRef) { if is_module_dict(obj) { panic!( "w_dict_set_strategy: W_ModuleDictObject strategy swap is not the canonical \ @@ -1246,7 +1253,7 @@ pub fn w_dict_new() -> PyObjectRef { w_class: get_instantiate(&DICT_TYPE), }, dstorage: entries as *mut u8, - dstrategy: &crate::dictmultiobject::EMPTY_DICT_STRATEGY, + dstrategy: &crate::dictmultiobject::EMPTY_DICT_STRATEGY_REF, keys_version: 0, clear_gen: 0, }, @@ -1268,7 +1275,7 @@ pub fn w_dict_new_kwargs() -> PyObjectRef { w_class: get_instantiate(&DICT_TYPE), }, dstorage: std::ptr::null_mut(), - dstrategy: &crate::dictmultiobject::EMPTY_KWARGS_DICT_STRATEGY, + dstrategy: &crate::dictmultiobject::EMPTY_KWARGS_DICT_STRATEGY_REF, keys_version: 0, clear_gen: 0, }, @@ -1282,10 +1289,7 @@ pub fn w_dict_new_kwargs() -> PyObjectRef { /// to allocate a fresh W_DictObject that preserves the source's /// strategy + a freshly cloned typed storage box. Length is computed /// on demand by `strategy.length(self)` from the typed storage shape. -pub fn w_dict_new_with( - strategy: &'static dyn crate::dictmultiobject::DictStrategy, - dstorage: *mut u8, -) -> PyObjectRef { +pub fn w_dict_new_with(strategy: &'static DictStrategyRef, dstorage: *mut u8) -> PyObjectRef { alloc_dict_object( W_DictObject { ob_header: PyObject { @@ -1320,7 +1324,7 @@ pub fn w_dict_new_unmanaged_side_table_value() -> PyObjectRef { w_class: get_instantiate(&DICT_TYPE), }, dstorage: entries as *mut u8, - dstrategy: &crate::dictmultiobject::OBJECT_DICT_STRATEGY, + dstrategy: &crate::dictmultiobject::OBJECT_DICT_STRATEGY_REF, keys_version: 0, clear_gen: 0, }) as PyObjectRef @@ -1518,13 +1522,11 @@ impl W_DictMultiObject for W_ModuleDictObject { /// `w_dict.dstorage = strategy.erase(d_new)`). The trait method /// hides that adapter from callers; the side-field layout retires /// alongside typed-strategy storage migration. - fn set_strategy(&mut self, strategy: &'static dyn crate::dictmultiobject::DictStrategy) { - let target = - strategy as *const dyn crate::dictmultiobject::DictStrategy as *const () as usize; - let object_singleton = &crate::dictmultiobject::OBJECT_DICT_STRATEGY - as *const crate::dictmultiobject::ObjectDictStrategy - as *const () as usize; - if target != object_singleton { + fn set_strategy(&mut self, strategy: &'static DictStrategyRef) { + // Distinct holders have distinct addresses; the singletons they carry + // are zero-sized, and comparing *those* addresses is not a valid + // identity test. + if !std::ptr::eq(strategy, &crate::dictmultiobject::OBJECT_DICT_STRATEGY_REF) { panic!( "W_ModuleDictObject::set_strategy: only ObjectDictStrategy transition is \ implemented (celldict.py:173-186 is the only documented swap target)" @@ -1975,7 +1977,7 @@ pub unsafe fn w_dict_strategy_id(obj: PyObjectRef) -> usize { // `&'static dyn DictStrategy` — the fat pointer carries both a // vtable and a data pointer; the data pointer alone uniquely // identifies the strategy singleton (`OBJECT_DICT_STRATEGY` etc.). - let raw: *const dyn crate::dictmultiobject::DictStrategy = d.dstrategy; + let raw: *const dyn crate::dictmultiobject::DictStrategy = d.dstrategy.imp; raw as *const () as usize } @@ -2381,7 +2383,7 @@ pub unsafe fn w_dict_is_empty_strategy(obj: PyObjectRef) -> bool { if is_module_dict(obj) { return false; } - let strategy = (*(obj as *const W_DictObject)).dstrategy; + let strategy = (*(obj as *const W_DictObject)).dstrategy.imp; strategy_is(strategy, &crate::dictmultiobject::EMPTY_DICT_STRATEGY) || strategy_is( strategy, @@ -2403,7 +2405,7 @@ pub unsafe fn w_dict_lookup_checked( if is_module_dict(obj) { return w_module_dict_lookup_inner_checked(obj, key); } - let strategy = (*(obj as *const W_DictObject)).dstrategy; + let strategy = (*(obj as *const W_DictObject)).dstrategy.imp; if strategy_is(strategy, &crate::dictmultiobject::EMPTY_DICT_STRATEGY) || strategy_is( strategy, @@ -2433,7 +2435,7 @@ pub unsafe fn w_dict_lookup_checked( if _never_equal_to_string(key) { return Ok(None); } - w_dict_set_strategy(obj, &crate::dictmultiobject::OBJECT_DICT_STRATEGY); + w_dict_set_strategy(obj, &crate::dictmultiobject::OBJECT_DICT_STRATEGY_REF); return w_dict_lookup_object_strategy_checked(obj, key); } if strategy_is(strategy, &crate::dictmultiobject::INT_DICT_STRATEGY) { @@ -2761,7 +2763,7 @@ unsafe fn w_dict_store_checked_inner( if is_module_dict(obj) { return w_module_dict_store_inner_checked(obj, key, value); } - let strategy = (*(obj as *const W_DictObject)).dstrategy; + let strategy = (*(obj as *const W_DictObject)).dstrategy.imp; if strategy_is(strategy, &crate::dictmultiobject::EMPTY_DICT_STRATEGY) { crate::dictmultiobject::EMPTY_DICT_STRATEGY.switch_to_correct_strategy(obj, key); return w_dict_store_checked_inner(obj, key, value, hash); @@ -2786,7 +2788,7 @@ unsafe fn w_dict_store_checked_inner( } if strategy_is(strategy, &crate::dictmultiobject::UNICODE_DICT_STRATEGY) { if !crate::is_exact_type(key, &crate::STR_TYPE) { - w_dict_set_strategy(obj, &crate::dictmultiobject::OBJECT_DICT_STRATEGY); + w_dict_set_strategy(obj, &crate::dictmultiobject::OBJECT_DICT_STRATEGY_REF); } return w_dict_store_object_strategy_checked_inner(obj, key, value, hash); } @@ -2867,7 +2869,7 @@ pub unsafe fn w_dict_setdefault_checked( w_module_dict_store_inner_checked(obj, key, value)?; return Ok(value); } - let strategy = (*(obj as *const W_DictObject)).dstrategy; + let strategy = (*(obj as *const W_DictObject)).dstrategy.imp; // `dictmultiobject.py:749-753 EmptyDictStrategy.setdefault`: // self.switch_to_correct_strategy(w_dict, w_key) // w_dict.setitem(w_key, w_default) @@ -2980,7 +2982,7 @@ pub unsafe fn w_dict_pop_checked( None => Ok(None), } } else { - let strategy = (*(obj as *const W_DictObject)).dstrategy; + let strategy = (*(obj as *const W_DictObject)).dstrategy.imp; if strategy_is(strategy, &crate::dictmultiobject::OBJECT_DICT_STRATEGY) { // `AbstractTypedStrategy.pop` (`dictmultiobject.py:1123`) performs // one r_dict lookup followed by removal. Run that probe @@ -3483,7 +3485,7 @@ pub unsafe fn w_dict_delitem_checked( if is_module_dict(obj) { return w_module_dict_delitem_inner_checked(obj, key); } - let strategy = (*(obj as *const W_DictObject)).dstrategy; + let strategy = (*(obj as *const W_DictObject)).dstrategy.imp; if strategy_is(strategy, &crate::dictmultiobject::EMPTY_DICT_STRATEGY) || strategy_is( strategy, @@ -3507,7 +3509,7 @@ pub unsafe fn w_dict_delitem_checked( if crate::is_exact_type(key, &crate::STR_TYPE) { return w_dict_delitem_object_strategy_checked(obj, key); } - w_dict_set_strategy(obj, &crate::dictmultiobject::OBJECT_DICT_STRATEGY); + w_dict_set_strategy(obj, &crate::dictmultiobject::OBJECT_DICT_STRATEGY_REF); return w_dict_delitem_object_strategy_checked(obj, key); } if strategy_is(strategy, &crate::dictmultiobject::INT_DICT_STRATEGY) { @@ -3574,7 +3576,7 @@ pub unsafe fn w_dict_delitem_if_value_is_checked( return Err(DictKeyError); } - let strategy = (*(obj as *const W_DictObject)).dstrategy; + let strategy = (*(obj as *const W_DictObject)).dstrategy.imp; if strategy.strategy_kind() != StrategyKind::Object { strategy.switch_to_object_strategy(obj); } @@ -3709,7 +3711,7 @@ pub unsafe fn w_dict_move_to_end_checked( return Err(DictKeyError); } - let strategy = (*(obj as *const W_DictObject)).dstrategy; + let strategy = (*(obj as *const W_DictObject)).dstrategy.imp; // `EmptyDictStrategy.move_to_end` (dictmultiobject.py:820-821) raises // `KeyError` for the requested key without hashing it: an unhashable key // reports `KeyError`, not the object strategy's unhashable `TypeError`, and @@ -4223,7 +4225,7 @@ pub unsafe fn w_dict_switch_int_to_object_strategy(w_dict: PyObjectRef) { dict.dstorage = crate::gc_storage::gc_alloc_storage_box(new_map, object_dict_storage_gc_type_id()) as *mut u8; - dict.dstrategy = &OBJECT_DICT_STRATEGY; + dict.dstrategy = &OBJECT_DICT_STRATEGY_REF; } /// Internal helper: `BytesDictStrategy::setitem` body — @@ -4361,7 +4363,7 @@ pub unsafe fn w_dict_switch_bytes_to_object_strategy(w_dict: PyObjectRef) { dict.dstorage = crate::gc_storage::gc_alloc_storage_box(new_map, object_dict_storage_gc_type_id()) as *mut u8; - dict.dstrategy = &OBJECT_DICT_STRATEGY; + dict.dstrategy = &OBJECT_DICT_STRATEGY_REF; } /// Internal helper: `ObjectDictStrategy::items` body for pyre's @@ -5126,7 +5128,7 @@ pub trait DictStrategy { /// `w_dict` must point at a valid `W_DictObject` whose /// `dstrategy` is `self`. unsafe fn switch_to_object_strategy(&self, w_dict: PyObjectRef) { - crate::dictmultiobject::w_dict_set_strategy(w_dict, &OBJECT_DICT_STRATEGY); + crate::dictmultiobject::w_dict_set_strategy(w_dict, &OBJECT_DICT_STRATEGY_REF); } /// `dictmultiobject.py:559-560 listview_bytes` — default returns @@ -5200,6 +5202,43 @@ pub trait DictStrategy { } } +/// The one-word strategy slot a dict stores, holding the trait object that +/// does the dispatch. +/// +/// PyPy's `dstrategy` is one instance pointer and the strategy's *type* lives +/// in the instance header, which is what the JIT's `guard_class` reads when it +/// inlines `self.get_strategy().getitem(...)`. A Rust `&dyn` inverts that — +/// the vtable rides in the reference — so a dict holding one directly has no +/// field to guard, and its data word is the address of a zero-sized static, +/// which Rust is free to make identical across distinct strategies. Boxing +/// the trait object in this `#[repr(C)]` holder restores both properties: the +/// dict slot is a single pointer, and each strategy's holder is a distinct, +/// non-zero-sized address. +/// +/// It derefs to `dyn DictStrategy`, so `dict.dstrategy.getitem(..)` and every +/// other call through the slot reads exactly as before. +#[repr(C)] +pub struct DictStrategyRef { + pub imp: &'static dyn crate::dictmultiobject::DictStrategy, +} + +/// The holders below wrap only the stateless unit-struct singletons, which are +/// shared process-wide exactly as `space.fromcache(StrategyCls)` shares them +/// upstream. `DictStrategy` itself cannot carry a `Sync` bound — +/// `ModuleDictStrategy`'s `GlobalCache` holds `*mut PyObject` — and a module +/// dict never uses a holder anyway: its strategy lives behind +/// `W_ModuleDictObject.mstrategy`. +unsafe impl Sync for DictStrategyRef {} + +impl std::ops::Deref for DictStrategyRef { + type Target = dyn crate::dictmultiobject::DictStrategy; + + #[inline] + fn deref(&self) -> &Self::Target { + self.imp + } +} + /// `pypy/objspace/std/dictmultiobject.py:1195+ ObjectDictStrategy` /// process-wide singleton. PyPy's `space.fromcache(ObjectDictStrategy)` /// returns the same instance for every space, and `W_DictObject`'s @@ -5235,6 +5274,28 @@ pub static UNICODE_DICT_STRATEGY: UnicodeDictStrategy = UnicodeDictStrategy; /// singleton. pub static INT_DICT_STRATEGY: IntDictStrategy = IntDictStrategy; +/// The [`DictStrategyRef`] holders a dict's `dstrategy` slot points at — one +/// per singleton above. Each is a distinct address; the singletons themselves +/// are zero-sized and need not be. +pub static OBJECT_DICT_STRATEGY_REF: DictStrategyRef = DictStrategyRef { + imp: &OBJECT_DICT_STRATEGY, +}; +pub static EMPTY_DICT_STRATEGY_REF: DictStrategyRef = DictStrategyRef { + imp: &EMPTY_DICT_STRATEGY, +}; +pub static EMPTY_KWARGS_DICT_STRATEGY_REF: DictStrategyRef = DictStrategyRef { + imp: &EMPTY_KWARGS_DICT_STRATEGY, +}; +pub static BYTES_DICT_STRATEGY_REF: DictStrategyRef = DictStrategyRef { + imp: &BYTES_DICT_STRATEGY, +}; +pub static UNICODE_DICT_STRATEGY_REF: DictStrategyRef = DictStrategyRef { + imp: &UNICODE_DICT_STRATEGY, +}; +pub static INT_DICT_STRATEGY_REF: DictStrategyRef = DictStrategyRef { + imp: &INT_DICT_STRATEGY, +}; + /// `dictmultiobject.py:684-790 EmptyDictStrategy`. /// /// ```python @@ -5277,10 +5338,7 @@ pub struct EmptyDictStrategy; /// RPython's `w_dict.dstorage = strategy.erase(storage)` is a `setfield_gc`: /// the GC transform roots and reloads `w_dict` across the storage allocation, /// then records the old-to-young edge. Do the same explicitly here. -unsafe fn install_empty_strategy( - w_dict: PyObjectRef, - strategy: &'static dyn crate::dictmultiobject::DictStrategy, -) { +unsafe fn install_empty_strategy(w_dict: PyObjectRef, strategy: &'static DictStrategyRef) { let _roots = crate::gc_roots::push_roots(); let dict_slot = crate::gc_roots::shadow_stack_len(); crate::gc_roots::pin_root(w_dict); @@ -5311,7 +5369,7 @@ impl EmptyDictStrategy { // `:696-698 type(w_key) is self.space.UnicodeObjectCls` // (Python 2 unicode / Python 3 str). if crate::is_exact_type(w_key, &crate::STR_TYPE) { - install_empty_strategy(w_dict, &UNICODE_DICT_STRATEGY); + install_empty_strategy(w_dict, &UNICODE_DICT_STRATEGY_REF); return; } // `:700-701 is_w(w_type, self.space.w_int)` — plain int only; @@ -5356,7 +5414,7 @@ impl EmptyDictStrategy { /// `w_dict` must point at a valid `W_DictObject` whose strategy /// is currently `EmptyDictStrategy`. unsafe fn switch_to_int_strategy(&self, w_dict: PyObjectRef) { - install_empty_strategy(w_dict, &INT_DICT_STRATEGY); + install_empty_strategy(w_dict, &INT_DICT_STRATEGY_REF); } /// `dictmultiobject.py:707-711 switch_to_bytes_strategy`: @@ -5375,7 +5433,7 @@ impl EmptyDictStrategy { /// # Safety /// Same as [`switch_to_int_strategy`]. unsafe fn switch_to_bytes_strategy(&self, w_dict: PyObjectRef) { - install_empty_strategy(w_dict, &BYTES_DICT_STRATEGY); + install_empty_strategy(w_dict, &BYTES_DICT_STRATEGY_REF); } /// `dictmultiobject.py:725-730 switch_to_identity_strategy`: @@ -5397,7 +5455,7 @@ impl EmptyDictStrategy { /// # Safety /// Same as [`switch_to_int_strategy`]. unsafe fn switch_to_identity_strategy(&self, w_dict: PyObjectRef) { - install_empty_strategy(w_dict, &crate::identitydict::IDENTITY_DICT_STRATEGY); + install_empty_strategy(w_dict, &crate::identitydict::IDENTITY_DICT_STRATEGY_REF); } } @@ -5429,7 +5487,7 @@ impl EmptyKwargsDictStrategy { /// `w_dict` must be a W_DictObject whose strategy is /// `EMPTY_KWARGS_DICT_STRATEGY`. unsafe fn switch_to_kwargs_strategy(&self, w_dict: PyObjectRef) { - install_empty_strategy(w_dict, &crate::kwargsdict::KWARGS_DICT_STRATEGY); + install_empty_strategy(w_dict, &crate::kwargsdict::KWARGS_DICT_STRATEGY_REF); } /// `dictmultiobject.py:692-705 switch_to_correct_strategy` @@ -5596,7 +5654,7 @@ impl DictStrategy for EmptyDictStrategy { /// pointer. The field overwrite is a `setfield_gc`; the unreachable /// old placeholder box is reclaimed by the sweep. unsafe fn switch_to_object_strategy(&self, w_dict: PyObjectRef) { - install_empty_strategy(w_dict, &OBJECT_DICT_STRATEGY); + install_empty_strategy(w_dict, &OBJECT_DICT_STRATEGY_REF); } unsafe fn getitem(&self, _w_dict: PyObjectRef, w_key: PyObjectRef) -> Option { @@ -5656,7 +5714,7 @@ impl DictStrategy for EmptyDictStrategy { // w_dict.setitem_str(key, w_value) // Unicode-strategy promotion is direct since the caller has // already chosen the str-keyed path. - install_empty_strategy(w_dict, &UNICODE_DICT_STRATEGY); + install_empty_strategy(w_dict, &UNICODE_DICT_STRATEGY_REF); crate::dictmultiobject::w_dict_setitem_str(w_dict, key, w_value); } @@ -5867,7 +5925,7 @@ impl DictStrategy for ObjectDictStrategy { storage.clone(), crate::dictmultiobject::object_dict_storage_gc_type_id(), ); - crate::dictmultiobject::w_dict_new_with(&OBJECT_DICT_STRATEGY, new_storage as *mut u8) + crate::dictmultiobject::w_dict_new_with(&OBJECT_DICT_STRATEGY_REF, new_storage as *mut u8) } } @@ -6040,7 +6098,7 @@ impl DictStrategy for BytesDictStrategy { storage.clone(), crate::dictmultiobject::bytes_dict_storage_gc_type_id(), ); - crate::dictmultiobject::w_dict_new_with(&BYTES_DICT_STRATEGY, new_storage as *mut u8) + crate::dictmultiobject::w_dict_new_with(&BYTES_DICT_STRATEGY_REF, new_storage as *mut u8) } } @@ -6109,7 +6167,7 @@ impl DictStrategy for UnicodeDictStrategy { if crate::dictmultiobject::_never_equal_to_string(w_key) { return None; } - crate::dictmultiobject::w_dict_set_strategy(w_dict, &OBJECT_DICT_STRATEGY); + crate::dictmultiobject::w_dict_set_strategy(w_dict, &OBJECT_DICT_STRATEGY_REF); crate::dictmultiobject::w_dict_lookup(w_dict, w_key) } @@ -6144,7 +6202,7 @@ impl DictStrategy for UnicodeDictStrategy { crate::dictmultiobject::w_dict_store_object_strategy(w_dict, w_key, w_value); return; } - crate::dictmultiobject::w_dict_set_strategy(w_dict, &OBJECT_DICT_STRATEGY); + crate::dictmultiobject::w_dict_set_strategy(w_dict, &OBJECT_DICT_STRATEGY_REF); crate::dictmultiobject::w_dict_store(w_dict, w_key, w_value); } @@ -6153,7 +6211,7 @@ impl DictStrategy for UnicodeDictStrategy { if crate::is_exact_type(w_key, &crate::STR_TYPE) { return crate::dictmultiobject::w_dict_delitem_object_strategy(w_dict, w_key); } - crate::dictmultiobject::w_dict_set_strategy(w_dict, &OBJECT_DICT_STRATEGY); + crate::dictmultiobject::w_dict_set_strategy(w_dict, &OBJECT_DICT_STRATEGY_REF); crate::dictmultiobject::w_dict_delitem(w_dict, w_key) } @@ -6237,7 +6295,7 @@ impl DictStrategy for UnicodeDictStrategy { storage.clone(), crate::dictmultiobject::object_dict_storage_gc_type_id(), ); - crate::dictmultiobject::w_dict_new_with(&UNICODE_DICT_STRATEGY, new_storage as *mut u8) + crate::dictmultiobject::w_dict_new_with(&UNICODE_DICT_STRATEGY_REF, new_storage as *mut u8) } } @@ -6418,7 +6476,7 @@ impl DictStrategy for IntDictStrategy { storage.clone(), crate::dictmultiobject::int_dict_storage_gc_type_id(), ); - crate::dictmultiobject::w_dict_new_with(&INT_DICT_STRATEGY, new_storage as *mut u8) + crate::dictmultiobject::w_dict_new_with(&INT_DICT_STRATEGY_REF, new_storage as *mut u8) } } diff --git a/pyre/pyre-object/src/identitydict.rs b/pyre/pyre-object/src/identitydict.rs index 712eee3e8db..79933970735 100644 --- a/pyre/pyre-object/src/identitydict.rs +++ b/pyre/pyre-object/src/identitydict.rs @@ -12,7 +12,7 @@ #![allow(unsafe_op_in_unsafe_fn)] -use crate::dictmultiobject::{DictStrategy, OBJECT_DICT_STRATEGY}; +use crate::dictmultiobject::DictStrategy; use crate::pyobject::PyObjectRef; /// `identitydict.py:12-83 IdentityDictStrategy` key type — identity @@ -170,7 +170,7 @@ pub unsafe fn w_dict_switch_identity_to_object_strategy(w_dict: PyObjectRef) { new_map, crate::dictmultiobject::object_dict_storage_gc_type_id(), ) as *mut u8; - dict.dstrategy = &OBJECT_DICT_STRATEGY; + dict.dstrategy = &crate::dictmultiobject::OBJECT_DICT_STRATEGY_REF; } #[inline] @@ -213,6 +213,13 @@ pub struct IdentityDictStrategy; /// singleton — matches PyPy's `space.fromcache(IdentityDictStrategy)`. pub static IDENTITY_DICT_STRATEGY: IdentityDictStrategy = IdentityDictStrategy; +/// The [`crate::dictmultiobject::DictStrategyRef`] holder a dict's `dstrategy` +/// slot points at. +pub static IDENTITY_DICT_STRATEGY_REF: crate::dictmultiobject::DictStrategyRef = + crate::dictmultiobject::DictStrategyRef { + imp: &IDENTITY_DICT_STRATEGY, + }; + impl IdentityDictStrategy { /// `identitydict.py:36-37 IdentityDictStrategy.is_correct_type` — /// `self.space.type(w_obj).compares_by_identity()`. Dispatch @@ -346,7 +353,7 @@ impl DictStrategy for IdentityDictStrategy { storage.clone(), identity_dict_storage_gc_type_id(), ); - crate::dictmultiobject::w_dict_new_with(&IDENTITY_DICT_STRATEGY, new_storage as *mut u8) + crate::dictmultiobject::w_dict_new_with(&IDENTITY_DICT_STRATEGY_REF, new_storage as *mut u8) } /// `pypy.objspace.std.identitydict.IdentityDictStrategy` stores diff --git a/pyre/pyre-object/src/kwargsdict.rs b/pyre/pyre-object/src/kwargsdict.rs index d219514af8f..0caeb224ccc 100644 --- a/pyre/pyre-object/src/kwargsdict.rs +++ b/pyre/pyre-object/src/kwargsdict.rs @@ -15,7 +15,7 @@ #![allow(unsafe_op_in_unsafe_fn)] -use crate::dictmultiobject::{DictStrategy, OBJECT_DICT_STRATEGY}; +use crate::dictmultiobject::DictStrategy; use crate::pyobject::PyObjectRef; /// `kwargsdict.py:25-178 KwargsDictStrategy`. @@ -53,6 +53,13 @@ pub struct KwargsDictStrategy; /// singleton — matches PyPy's `space.fromcache(KwargsDictStrategy)`. pub static KWARGS_DICT_STRATEGY: KwargsDictStrategy = KwargsDictStrategy; +/// The [`crate::dictmultiobject::DictStrategyRef`] holder a dict's `dstrategy` +/// slot points at. +pub static KWARGS_DICT_STRATEGY_REF: crate::dictmultiobject::DictStrategyRef = + crate::dictmultiobject::DictStrategyRef { + imp: &KWARGS_DICT_STRATEGY, + }; + /// `KwargsDictStrategy` backing — erased `([], [])` parallel arrays /// (`kwargsdict.py:27-29`). GC-managed storage box (mirrors the other /// dict strategies; see `dictmultiobject::ObjectDictStorage`). @@ -101,7 +108,7 @@ pub unsafe fn w_dict_switch_kwargs_to_object_strategy(w_dict: PyObjectRef) { new_map, crate::dictmultiobject::object_dict_storage_gc_type_id(), ) as *mut u8; - dict.dstrategy = &OBJECT_DICT_STRATEGY; + dict.dstrategy = &crate::dictmultiobject::OBJECT_DICT_STRATEGY_REF; } /// `kwargsdict.py:62` size threshold past which the strategy @@ -150,7 +157,7 @@ impl KwargsDictStrategy { let keys_w = std::mem::take(&mut old.0); let values_w = std::mem::take(&mut old.1); dict.dstorage = crate::dictmultiobject::UNICODE_DICT_STRATEGY.get_empty_storage(); - dict.dstrategy = &crate::dictmultiobject::UNICODE_DICT_STRATEGY; + dict.dstrategy = &crate::dictmultiobject::UNICODE_DICT_STRATEGY_REF; for (k, v) in keys_w.into_iter().zip(values_w.into_iter()) { crate::dictmultiobject::w_dict_store(w_dict, k, v); } @@ -307,6 +314,6 @@ impl DictStrategy for KwargsDictStrategy { storage.clone(), kwargs_dict_storage_gc_type_id(), ); - crate::dictmultiobject::w_dict_new_with(&KWARGS_DICT_STRATEGY, new_storage as *mut u8) + crate::dictmultiobject::w_dict_new_with(&KWARGS_DICT_STRATEGY_REF, new_storage as *mut u8) } } From d4dc416084efb30a1d60c09ab362799c302f78e0 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 6 Aug 2026 03:40:57 +0900 Subject: [PATCH 06/10] jit: give the dict.lookup oopspec a producer, so a str subscript on a unicode-strategy dict folds `OS_DICT_LOOKUP` and its optimizer arm `_optimize_call_dict_lookup` (`optimizeopt/heap.rs`) were ported but had no producer anywhere in `pyre/`: no recorder emitted a call carrying `OopSpecIndex::DictLookup`, so the arm was dead code and every `d[k]` residualized through the generic subscript path with `RandomEffects`. Add the recorder arm. `try_walker_specialize_subscr` now takes a dict leg when the receiver is a canonical `W_DictObject` on `StrategyKind::Unicode`, the key is an exact canonical `str`, and the concrete probe hits. It emits exact class/`w_class` guards on both operands, a `GuardValue` on the one-word `dstrategy` slot pinning `UNICODE_DICT_STRATEGY_REF`, an elidable `ll_strhash` call for the key digest, the `dict.lookup` call itself, `IntGe`/`GuardTrue` on the returned index, and a `GuardNonnull`-protected value read. The lookup call is `EF_CANNOT_RAISE`. What makes that honest is the strategy guard: `UnicodeDictStrategy::setitem` hands the dict to `OBJECT_DICT_STRATEGY_REF` the moment a non-exact-`str` key is stored, so while the guard holds every stored key is an exact `str` and the comparisons are WTF-8 byte equality. `w_dict_unicode_lookup_index` enforces rather than assumes this: it runs the probe inside the callback-free bracket, and a key pair the builtin ladder cannot decide breaks the probe and reports a miss, which side-exits to the generic residual. `extraeffect < 6` is what the recorded op diff shows: `ForceToken`, both vable/`py_pc` `SetfieldGc` stamps, `GuardNotForced` and `GuardNoException` are all gone from the compiled loop. Supporting pieces: * `W_DICT_DESCR_GROUP` gains `dstrategy_word` and `dstorage_lookup_ns`, appended after `keys_version` so the existing census index 0 is unmoved. Neither key is spelled as its Rust field name: `get_field_descr` caches on `(struct_key, field_name)` and the LLBC analyzer resolves field accesses by name, so a census key that collides with a real field would hand the analyzer whichever type won the LazyLock race. * `dict_lookup_entries_array_descr`, the second `extradescrs` member the optimizer arm requires. * `jit_dict_value_at`, the index-to-value read. * Both `pyre-object` helpers take `*mut PyObject` rather than `PyObjectRef`: `dont_look_inside` decides syntactically whether to emit the `__majit_call_target_*` word-ABI trampoline, and an alias path is not a raw-pointer token. `jit_fnaddr` registers the trampolines, not the raw functions, so the baked funcbox address matches the registered one on wasm. * `w_dict_unicode_lookup_index` holds the dict's own lock over the table read, like every other reader of `dstorage`. Measured, median of pairwise ratios over 6 alternating rounds: `same_key` 0.673, `two_keys` 0.697, `fresh_key` 0.916 (its cost is the str-slice allocation the fold does not touch). jit-stats for the folded loop: `loops_compiled=1 bridges_compiled=0 loops_aborted=0 guard_failures=1`. `_optimize_call_dict_lookup` does not remove the loop body's lookup: the peeled preamble and the body are optimized separately, so `cached_dict_reads` does not cross the label. Upstream behaves the same way. Assisted-by: Claude --- majit/majit-backend-wasm/src/codegen.rs | 2 +- .../parity_tests/dict_unicode_lookup_fold.py | 210 ++++++++++++++++++ pyre/pyre-interpreter/src/jit_fnaddr.rs | 25 +++ pyre/pyre-jit-trace/src/descr.rs | 83 ++++++- pyre/pyre-jit-trace/src/helpers.rs | 22 ++ .../src/jitcode_dispatch/specialize.rs | 162 +++++++++++++- pyre/pyre-object/src/dictmultiobject.rs | 96 ++++++++ 7 files changed, 587 insertions(+), 13 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/dict_unicode_lookup_fold.py diff --git a/majit/majit-backend-wasm/src/codegen.rs b/majit/majit-backend-wasm/src/codegen.rs index cd5ca144943..4f190cd899c 100644 --- a/majit/majit-backend-wasm/src/codegen.rs +++ b/majit/majit-backend-wasm/src/codegen.rs @@ -1313,7 +1313,7 @@ fn collect_guards_and_vars(inputargs: &[InputArg], ops: &[Op]) -> (Vec *max_var { *max_var = a.raw() + 1; } diff --git a/pyre/extra_tests/parity_tests/dict_unicode_lookup_fold.py b/pyre/extra_tests/parity_tests/dict_unicode_lookup_fold.py new file mode 100644 index 00000000000..050bc3354ec --- /dev/null +++ b/pyre/extra_tests/parity_tests/dict_unicode_lookup_fold.py @@ -0,0 +1,210 @@ +# `d[key]` on a str-keyed dict, run hot enough to be traced. +# +# The subscript fold pins the dict to the Unicode strategy and the key to the +# canonical `str` class, then records `rstr.ll_strhash` as an elidable call and +# the probe itself as the `rordereddict dict.lookup` oopspec. The optimizer is +# allowed to drop a repeat of the same `(dict, key)` probe, so the entry index a +# compiled loop uses can come from an EARLIER iteration. Every case below is +# built so a wrongly-retained index shows up as a wrong value rather than as a +# missing crash. + +N = 400 + + +def check(got, want, label): + assert got == want, "%s: %r != %r" % (label, got, want) + + +# ── the plain shape: one loop-invariant dict, one loop-invariant key ── +def repeat_same_key(n): + d = {"tag": 3, "other": 5} + total = 0 + i = 0 + while i < n: + total = total + d["tag"] + i = i + 1 + return total + + +# ── keys that are EQUAL but not the same object. The identity-keyed +# `dict.get` fold could never reach this; the hash+equality probe must ── +def fresh_key_each_iteration(n): + d = {"tag": 7} + total = 0 + i = 0 + while i < n: + k = ("tagX")[:-1] + total = total + d[k] + i = i + 1 + return total + + +# ── the value under one key is rewritten every iteration. A value-only +# overwrite keeps the entry index, so the fold may keep it — but the VALUE read +# must stay live, never folded to the recorded one ── +def value_overwritten(n): + d = {"acc": 0} + i = 0 + while i < n: + d["acc"] = d["acc"] + i + i = i + 1 + return d["acc"] + + +# ── the KEY SET changes under the loop. Each insert can move the entry a +# retained index points at, so a stale index would read the wrong value ── +def key_set_grows(n): + d = {"probe": 1} + total = 0 + i = 0 + while i < n: + d["k%d" % i] = i + total = total + d["probe"] + i = i + 1 + return total + + +# ── a delete compacts the table, renumbering every entry after the hole ── +def key_set_shrinks(n): + d = {} + i = 0 + while i < 64: + d["k%d" % i] = i + i = i + 1 + d["probe"] = 99 + total = 0 + i = 0 + while i < n: + victim = "k%d" % (i % 64) + if victim in d: + del d[victim] + total = total + d["probe"] + i = i + 1 + return total + + +# ── two dicts alternating at one call site: a receiver mix-up cross-checks ── +def two_receivers(n): + a = {"tag": 11} + b = {"tag": 22} + total = 0 + i = 0 + while i < n: + total = total + (a if i % 2 == 0 else b)["tag"] + i = i + 1 + return total + + +# ── the dict leaves the Unicode strategy mid-loop. Storing a non-str key +# promotes it to the Object strategy, so the strategy guard must side-exit and +# the same call site must keep answering correctly afterwards ── +def strategy_switches(n): + d = {"tag": 4} + total = 0 + i = 0 + while i < n: + if i == n // 2: + d[17] = 100 + total = total + d["tag"] + i = i + 1 + return total + d[17] + + +# ── a miss must raise KeyError, not answer a neighbouring entry ── +def miss_raises(n): + d = {"present": 1} + hits = 0 + misses = 0 + i = 0 + while i < n: + try: + hits = hits + d["present" if i % 3 else "absent"] + except KeyError: + misses = misses + 1 + i = i + 1 + return hits * 1000 + misses + + +# ── a dict SUBCLASS defines `__missing__`, so the fold must decline for it and +# the generic path must reach the override ── +class WithMissing(dict): + def __missing__(self, key): + return len(key) + + +def subclass_missing(n): + d = WithMissing() + d["here"] = 2 + total = 0 + i = 0 + while i < n: + total = total + d["here"] + d["absent"] + i = i + 1 + return total + + +# ── a str SUBCLASS key may override `__hash__`/`__eq__`, so it must not take +# the exact-str probe ── +class Shouty(str): + def __hash__(self): + return hash(str(self).lower()) + + def __eq__(self, other): + return str(self).lower() == str(other).lower() + + +def subclass_key(n): + d = {"tag": 6} + plain = 0 + shouty = 0 + i = 0 + while i < n: + plain = plain + d["tag"] + try: + shouty = shouty + d[Shouty("TAG")] + except KeyError: + shouty = shouty - 1 + i = i + 1 + return plain * 100000 + shouty + + +def run(): + check(repeat_same_key(N), 3 * N, "repeat_same_key") + check(fresh_key_each_iteration(N), 7 * N, "fresh_key_each_iteration") + check(value_overwritten(N), sum(range(N)), "value_overwritten") + check(key_set_grows(N), N, "key_set_grows") + check(key_set_shrinks(N), 99 * N, "key_set_shrinks") + check(two_receivers(N), (11 + 22) * (N // 2), "two_receivers") + check(strategy_switches(N), 4 * N + 100, "strategy_switches") + check(miss_raises(N), expected_miss(), "miss_raises") + check(subclass_missing(N), (2 + 6) * N, "subclass_missing") + check(subclass_key(N), expected_subclass_key(), "subclass_key") + + +# `miss_raises` / `subclass_key` checksums are written out from the same +# per-iteration rule rather than reusing the function, so a wrong answer cannot +# be cancelled by a matching wrong expectation. +def expected_miss(): + hits = 0 + misses = 0 + for i in range(N): + if i % 3: + hits = hits + 1 + else: + misses = misses + 1 + return hits * 1000 + misses + + +def expected_subclass_key(): + # `Shouty("TAG")` hashes as "tag" and compares equal to it, so every + # iteration finds the entry through the subclass's own protocol. + return (6 * N) * 100000 + 6 * N + + +run() + +# A second pass on already-warm code: every loop above has been traced by now, +# so this run executes the compiled form rather than building it. +run() + +print("OK") diff --git a/pyre/pyre-interpreter/src/jit_fnaddr.rs b/pyre/pyre-interpreter/src/jit_fnaddr.rs index 2f77d089b71..4226b6f6df4 100644 --- a/pyre/pyre-interpreter/src/jit_fnaddr.rs +++ b/pyre/pyre-interpreter/src/jit_fnaddr.rs @@ -2020,6 +2020,31 @@ pub fn jit_trace_fnaddrs() -> Vec<(&'static str, i64)> { "pyre_object::dict_entries_index_of_object", pyre_object::dictmultiobject::dict_entries_index_of_object as *const (), ); + // The `dict.lookup` producer's two residuals bind the macro-emitted + // `__majit_call_target_*` trampoline, not the raw fn. The wasm backend + // lowers a Ref/Int-result residual to a `call_indirect` whose static type + // comes from the descr alone — `(i64 x n) -> i64` — so a raw + // `(*mut PyObject, *mut PyObject, i64, i64) -> i64`, which is + // `(i32, i32, i64, i64) -> i64` on wasm32, traps + // `indirect call type mismatch`. The trampoline takes and returns the + // uniform machine word on every target. The raw fn stays reachable as + // `__majit_call_policy_*`'s null-target fallback. + let w_dict_unicode_lookup_index: extern "C" fn(i64, i64, i64, i64) -> i64 = + pyre_object::dictmultiobject::__majit_call_target_w_dict_unicode_lookup_index; + push_alias_pair( + &mut entries, + "pyre_object::dictmultiobject::w_dict_unicode_lookup_index", + "pyre_object::w_dict_unicode_lookup_index", + w_dict_unicode_lookup_index as *const (), + ); + let w_dict_unicode_key_hash: extern "C" fn(i64) -> i64 = + pyre_object::dictmultiobject::__majit_call_target_w_dict_unicode_key_hash; + push_alias_pair( + &mut entries, + "pyre_object::dictmultiobject::w_dict_unicode_key_hash", + "pyre_object::w_dict_unicode_key_hash", + w_dict_unicode_key_hash as *const (), + ); // The module-dict storage's own `String`-keyed probe / store pair. push_alias_pair( &mut entries, diff --git a/pyre/pyre-jit-trace/src/descr.rs b/pyre/pyre-jit-trace/src/descr.rs index 36cdd770daf..411d1564eb3 100644 --- a/pyre/pyre-jit-trace/src/descr.rs +++ b/pyre/pyre-jit-trace/src/descr.rs @@ -1196,20 +1196,50 @@ static FUNCTION_DESCR_GROUP: LazyLock = LazyLock::new(|| { /// bumps it; value-only replacement deliberately does not. A promoted /// identity-key lookup can therefore guard this field to pin the resolved /// entry index while continuing to read that entry's value live. +/// +/// The `dstrategy_word` and `dstorage_lookup_ns` keys deliberately do NOT match +/// the Rust field names they cover. `gc_cache().get_field_descr` is keyed by +/// `(struct_key, field_name)` and a cache HIT returns the cached descriptor with +/// the caller's declared type ignored, while the LLBC analyzer resolves a real +/// struct field access by name — so a key spelled `dstrategy` would share one +/// descriptor with the analyzer's own mint and whichever side initialised first +/// would decide the field's type for both. A distinct key is a distinct cache +/// slot, which is what keeps these raw-word views honest. They are appended +/// rather than placed in offset order so `keys_version` keeps census index 0. static W_DICT_DESCR_GROUP: LazyLock = LazyLock::new(|| { build_object_descr_group_with_def_path( pyre_object::dictmultiobject::W_DICT_OBJECT_SIZE, W_DICT_GC_TYPE_ID, &pyre_object::pyobject::DICT_TYPE as *const _ as usize, - &[( - "keys_version", - std::mem::offset_of!(pyre_object::dictmultiobject::W_DictObject, keys_version), - std::mem::size_of::(), - Type::Int, - false, - false, - false, - )], + &[ + ( + "keys_version", + std::mem::offset_of!(pyre_object::dictmultiobject::W_DictObject, keys_version), + std::mem::size_of::(), + Type::Int, + false, + false, + false, + ), + ( + "dstrategy_word", + std::mem::offset_of!(pyre_object::dictmultiobject::W_DictObject, dstrategy), + std::mem::size_of::(), + Type::Int, + false, + false, + false, + ), + ( + "dstorage_lookup_ns", + std::mem::offset_of!(pyre_object::dictmultiobject::W_DictObject, dstorage), + std::mem::size_of::(), + Type::Int, + false, + false, + false, + ), + ], "W_DictObject", "dictmultiobject::W_DictObject", ) @@ -2407,6 +2437,41 @@ pub fn dict_keys_version_descr() -> DescrRef { field_descr_from_group(&W_DICT_DESCR_GROUP, 0) } +/// `W_DictObject.dstrategy` as a raw word, for the `GuardValue` that pins a +/// dict to one strategy singleton. The census key is deliberately not the +/// struct field name — see the group's doc comment. +pub fn dict_strategy_word_descr() -> DescrRef { + field_descr_from_group(&W_DICT_DESCR_GROUP, 1) +} + +/// The cache-namespace half of the `dict.lookup` oopspec's `extradescrs` +/// (`heap.py:504-511 descrs[0]`), naming the entry table a lookup probes. +/// Only its identity is read; the slot is never loaded. +pub fn dict_lookup_namespace_descr() -> DescrRef { + field_descr_from_group(&W_DICT_DESCR_GROUP, 2) +} + +/// `extradescrs[1]` — the entry-array descr `_optimize_CALL_DICT_LOOKUP` +/// registers in `corresponding_array_descrs` so a call that writes the table +/// drops the cached lookups. No pyre helper declares it in +/// `write_descrs_arrays` yet, so its `ei_index` stays `u32::MAX` and +/// `check_write_descr_array` never fires for it; the cache is dropped instead +/// by `clean_caches`, which every dict-mutating residual triggers. +pub fn dict_lookup_entries_array_descr() -> DescrRef { + static DESCR: LazyLock = LazyLock::new(|| { + make_array_descr_with_full_id( + 0, + 8, + 0, + None, + Type::Ref, + false, + Some("dict.lookup.entries".to_string()), + ) + }); + DESCR.clone() +} + /// `Method.w_self` — the receiver object. The bound-method /// specialization extracts this via `GetfieldGcR` to recover the receiver /// `OpRef` after `LOAD_METHOD` discarded it (load_method.rs:6334 pushes diff --git a/pyre/pyre-jit-trace/src/helpers.rs b/pyre/pyre-jit-trace/src/helpers.rs index 9cfe4a60730..22278259714 100644 --- a/pyre/pyre-jit-trace/src/helpers.rs +++ b/pyre/pyre-jit-trace/src/helpers.rs @@ -142,6 +142,28 @@ pub extern "C" fn jit_dict_nth_value_versioned( } } +/// Read a dict value at an entry index a traced `dict.lookup` settled on — +/// `rordereddict.py:709 ll_dict_getitem`'s `d.entries[i].value` after +/// `ll_dict_lookup` returned the slot. The read goes through the live strategy +/// so a value-only overwrite stays visible, which is the whole reason the +/// lookup and the value read are two operations rather than one. +/// +/// The bounds check is load-bearing, not defensive. `_optimize_CALL_DICT_LOOKUP` +/// may delete a repeat of the same `(dict, key)` probe, so the index reaching +/// here can have been produced by an earlier iteration; an out-of-range index +/// answers `PY_NULL` and the caller's `GuardNonnull` side-exits to the generic +/// residual instead of indexing a compacted table. +pub extern "C" fn jit_dict_value_at(dict: i64, index: i64) -> i64 { + if index < 0 { + return PY_NULL as i64; + } + let dict = dict as PyObjectRef; + unsafe { + pyre_object::dictmultiobject::w_dict_nth_item(dict, index as usize) + .map_or(PY_NULL as i64, |(_, value)| value as i64) + } +} + pub fn emit_trace_call_ref_typed_elidable_cannot_raise( ctx: &mut TraceCtx, helper: *const (), diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs index 7eb59b8e349..95e2a5846a7 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs @@ -5148,9 +5148,14 @@ pub(crate) fn try_walker_specialize_binary_op_float( /// The authentic boxed result is taken from the same `execute_may_force_call` /// path the generic leg uses. /// -/// Tuples, empty-strategy lists, negative indices, and non-`list[int]` -/// operands fall through to the generic `CallMayForce` record (`Ok(None)`), -/// preserving Python `__getitem__` semantics. +/// A canonical Unicode-strategy dict with an exact-str key and a concrete hit +/// records the `rordereddict.py dict.lookup` oopspec producer: exact dict/key +/// guards, a strategy guard, elidable `rstr.ll_strhash`, `dict.lookup`, a +/// non-negative guard on the returned entry index, then a guarded value read. +/// +/// Tuples, dict misses, empty-strategy lists, negative indices, and +/// non-`list[int]` operands fall through to the generic `CallMayForce` record +/// (`Ok(None)`), preserving Python `__getitem__` semantics. pub(crate) fn try_walker_specialize_subscr( ctx: &mut WalkContext<'_, '_, Sym>, op_pc: usize, @@ -5201,6 +5206,157 @@ pub(crate) fn try_walker_specialize_subscr( ); } + // The `dict.lookup` gate. Both `w_class` checks are load-bearing: a dict + // SUBCLASS shares `ob_type == &DICT_TYPE` but retags `w_class` and reaches + // `__missing__` on a miss, and a str SUBCLASS key may override `__hash__` / + // `__eq__`, so neither may take the exact-str probe. The strategy check is + // what makes the probe non-raising: `UnicodeDictStrategy` hands the dict to + // `ObjectDictStrategy` the moment a non-exact-str key is stored, so while + // it holds, every stored key is an exact str and the comparisons are WTF-8 + // byte equality (`dictmultiobject.py:1286+` `r_dict(unicode_eq, + // unicode_hash)`). + let canonical_dict = pyre_object::pyobject::get_instantiate(&pyre_object::pyobject::DICT_TYPE); + let dict_unicode = !canonical_dict.is_null() + && unsafe { + std::ptr::eq((*list_obj).ob_type, &pyre_object::pyobject::DICT_TYPE) + && std::ptr::eq((*list_obj).w_class, canonical_dict) + && pyre_object::dictmultiobject::w_dict_get_strategy(list_obj).strategy_kind() + == pyre_object::dictmultiobject::StrategyKind::Unicode + }; + let canonical_str = if dict_unicode { + pyre_object::pyobject::get_instantiate(&pyre_object::pyobject::STR_TYPE) + } else { + std::ptr::null_mut() + }; + let dict_unicode_hit = !canonical_str.is_null() + && unsafe { + std::ptr::eq((*key_obj).ob_type, &pyre_object::pyobject::STR_TYPE) + && std::ptr::eq((*key_obj).w_class, canonical_str) + }; + if dict_unicode_hit { + let hash = unsafe { pyre_object::dictmultiobject::w_dict_unicode_key_hash(key_obj) }; + let index = unsafe { + pyre_object::dictmultiobject::w_dict_unicode_lookup_index(list_obj, key_obj, hash, 0) + }; + if index < 0 { + return Ok(None); + } + + let Some(boxed_result_i64) = walker_execute_may_force_boxed(ctx, allboxes, call_descr) + else { + return Ok(None); + }; + + walker_guard_class( + ctx, + op_pc, + list_op, + &pyre_object::pyobject::DICT_TYPE as *const _ as i64, + )?; + walker_guard_exact_w_class(ctx, op_pc, list_op, canonical_dict)?; + let strategy = crate::state::opimpl_getfield_gc_i( + ctx.trace_ctx, + list_op, + crate::descr::dict_strategy_word_descr(), + ); + let unicode_strategy_const = ctx + .trace_ctx + .const_int(&pyre_object::dictmultiobject::UNICODE_DICT_STRATEGY_REF as *const _ as i64); + ctx.trace_ctx + .record_guard(OpCode::GuardValue, &[strategy, unicode_strategy_const], 0); + walker_capture_snapshot_for_last_guard(ctx, op_pc)?; + ctx.trace_ctx + .heap_cache_mut() + .replace_box(strategy, unicode_strategy_const); + walker_guard_class( + ctx, + op_pc, + key_op, + &pyre_object::pyobject::STR_TYPE as *const _ as i64, + )?; + walker_guard_exact_w_class(ctx, op_pc, key_op, canonical_str)?; + + let hash_effect = majit_ir::EffectInfo::new( + majit_ir::ExtraEffect::ElidableCannotRaise, + majit_ir::OopSpecIndex::None, + ); + // Both residuals bind the macro-emitted `__majit_call_target_*` + // trampoline rather than the raw fn: the wasm backend derives a + // residual's `call_indirect` type from the descr alone — `(i64 x n) -> + // i64` — so a raw `*mut PyObject` argument, `i32` on wasm32, traps + // `indirect call type mismatch`. The trampoline takes and returns the + // uniform machine word everywhere, and is the address `jit_fnaddr` + // registers for these paths. + let hash_fn = { + let f: extern "C" fn(i64) -> i64 = + pyre_object::dictmultiobject::__majit_call_target_w_dict_unicode_key_hash; + f as *const () + }; + let hash_op = ctx.trace_ctx.call_typed_with_effect_pure( + OpCode::CallI, + hash_fn, + &[key_op], + &[majit_ir::Type::Ref], + majit_ir::Type::Int, + hash_effect, + &[ + majit_ir::Value::Int(hash_fn as i64), + majit_ir::Value::Ref(majit_ir::GcRef(key_obj as usize)), + ], + majit_ir::Value::Int(hash), + ); + + let mut lookup_effect = majit_ir::EffectInfo::new( + majit_ir::ExtraEffect::CannotRaise, + majit_ir::OopSpecIndex::DictLookup, + ); + lookup_effect.extradescrs = Some(vec![ + crate::descr::dict_lookup_namespace_descr(), + crate::descr::dict_lookup_entries_array_descr(), + ]); + let lookup_flag = ctx.trace_ctx.const_int(0); + let lookup_fn: extern "C" fn(i64, i64, i64, i64) -> i64 = + pyre_object::dictmultiobject::__majit_call_target_w_dict_unicode_lookup_index; + let index_op = ctx.trace_ctx.call_typed_with_effect( + OpCode::CallI, + lookup_fn as *const (), + &[list_op, key_op, hash_op, lookup_flag], + &[ + majit_ir::Type::Ref, + majit_ir::Type::Ref, + majit_ir::Type::Int, + majit_ir::Type::Int, + ], + majit_ir::Type::Int, + lookup_effect, + ); + ctx.trace_ctx + .set_opref_concrete(index_op, majit_ir::Value::Int(index)); + + let zero = ctx.trace_ctx.const_int(0); + let nonneg = ctx.trace_ctx.record_op(OpCode::IntGe, &[index_op, zero]); + ctx.trace_ctx + .set_opref_concrete(nonneg, majit_ir::Value::Int(1)); + walker_emit_guard_with_snapshot(ctx, op_pc, OpCode::GuardTrue, &[nonneg])?; + + let value = ctx.trace_ctx.call_ref_typed_with_effect( + crate::helpers::jit_dict_value_at as *const (), + &[list_op, index_op], + &[majit_ir::Type::Ref, majit_ir::Type::Int], + majit_ir::EffectInfo::new( + majit_ir::ExtraEffect::CannotRaise, + majit_ir::OopSpecIndex::None, + ), + ); + walker_emit_fold_guard_with_snapshot(ctx, op_pc, OpCode::GuardNonnull, &[value])?; + ctx.trace_ctx.set_opref_concrete( + value, + majit_ir::Value::Ref(majit_ir::GcRef(boxed_result_i64 as usize)), + ); + write_residual_call_result_to_dst(ctx, op_pc, dst, dst_bank, value)?; + return Ok(Some(())); + } + // Gate: EXACT list[int], non-negative index in bounds, int- or // float-storage. A bool index (`is_int` accepts `W_BoolObject`) is fine: // bool shares int's `intval`, so it unboxes through its own &BOOL_TYPE diff --git a/pyre/pyre-object/src/dictmultiobject.rs b/pyre/pyre-object/src/dictmultiobject.rs index e48299e8aea..7df5d55abd8 100644 --- a/pyre/pyre-object/src/dictmultiobject.rs +++ b/pyre/pyre-object/src/dictmultiobject.rs @@ -4394,6 +4394,76 @@ pub unsafe fn w_dict_nth_item_object_strategy( entries.get_index(index).map(|(k, &v)| (k.obj, v)) } +/// `rordereddict.py:46-48 ll_call_lookup_function` — the `dict.lookup` oopspec +/// residual, answering the entry index for `w_key` (or -1 for a miss) under a +/// hash the caller already computed. +/// +/// `flag` is the `FLAG_LOOKUP` / `FLAG_STORE` / `FLAG_DELETE` selector +/// `ll_dict_lookup` takes (`rordereddict.py:1041-1060`). An `IndexMap` keeps no +/// free-slot bookkeeping, so every flag answers the same index; the argument +/// exists because the optimizer reads it off `arg(4)` (`heap.py:497-500`). +/// +/// Only a Unicode-strategy dict reaches here, so every stored key is an exact +/// `str` and the probe's comparisons are WTF-8 byte equality. The +/// callback-free probe bracket enforces that: a pair that would need user code +/// breaks the probe and reports a miss, which side-exits the compiled trace to +/// the generic residual rather than running `__eq__` inside a call the recorder +/// promised cannot raise. +/// +/// The object parameters are spelled `*mut PyObject` rather than +/// [`PyObjectRef`]: `dont_look_inside` decides syntactically whether it can +/// emit the `__majit_call_target_*` word-ABI trampoline, and an alias path is +/// not a raw-pointer token, so the alias spelling silently yields no +/// trampoline — which is what the wasm backend's `call_indirect` needs. +/// +/// The table read holds the dict's own lock, like every other reader of +/// `dstorage`, so a concurrent `setitem` cannot resize the `IndexMap` under the +/// scan. Holding it across the probe cannot deadlock: `dict_keys_equal` runs no +/// user code while the bracket is active — a pair the builtin ladder cannot +/// decide breaks the probe instead — so nothing under the lock can re-enter a +/// dict operation. +/// +/// # Safety +/// `w_dict` must be a live `W_DictObject` on a strategy whose storage is an +/// `ObjectDictStorage`, and `hash` the digest `object_key_for` produces for +/// `w_key`. +#[majit_macros::dont_look_inside] +pub unsafe fn w_dict_unicode_lookup_index( + w_dict: *mut PyObject, + w_key: *mut PyObject, + hash: i64, + _flag: i64, +) -> i64 { + lock_dict_refs!(_dict_guard, w_dict, w_key); + let dict = &*(w_dict as *const W_DictObject); + let entries = &*(dict.dstorage as *const indexmap::IndexMap); + crate::dict_eq_hook::take_eq_error(); + crate::dict_eq_hook::begin_callback_free_probe(); + let found = entries.get_index_of(&ObjectKey { hash, obj: w_key }); + let probe_needed_user_code = crate::dict_eq_hook::end_callback_free_probe(); + match found { + Some(i) if !probe_needed_user_code => i as i64, + _ => -1, + } +} + +/// `rstr.ll_strhash` — the digest [`object_key_for`] caches for an exact `str` +/// key, isolated so a recorder that has already guarded the key's exact type +/// can emit it as one elidable call and let the optimizer hoist it out of a +/// loop. Memoized on the string object, content-derived and identity-stable, +/// which is what `@jit.elidable` asserts. +/// +/// `w_key` is spelled `*mut PyObject` for [`w_dict_unicode_lookup_index`]'s +/// reason. +/// +/// # Safety +/// `w_key` must be a live exact `str`. +#[majit_macros::dont_look_inside] +pub unsafe fn w_dict_unicode_key_hash(w_key: *mut PyObject) -> i64 { + crate::dict_eq_hook::try_hash_w(w_key) + .unwrap_or_else(|| crate::dict_eq_hook::missing_hash_hook()) +} + /// Internal helper: `ModuleDictStrategy::items` body for pyre's /// W_ModuleDictObject — branches on `is_object_strategy` and emits /// from whichever storage half is live. Called only from the @@ -6554,6 +6624,32 @@ mod tests { } } + #[test] + fn test_w_dict_unicode_lookup_index_helpers() { + install_test_hash_hook(); + let dict = w_dict_new(); + unsafe { + w_dict_setitem_str(dict, "alpha", w_int_new(1)); + w_dict_setitem_str(dict, "beta", w_int_new(2)); + + let alpha = w_str_new("alpha"); + let beta = w_str_new("beta"); + let gamma = w_str_new("gamma"); + assert_eq!( + w_dict_unicode_lookup_index(dict, alpha, w_dict_unicode_key_hash(alpha), 0), + 0, + ); + assert_eq!( + w_dict_unicode_lookup_index(dict, beta, w_dict_unicode_key_hash(beta), 0), + 1, + ); + assert_eq!( + w_dict_unicode_lookup_index(dict, gamma, w_dict_unicode_key_hash(gamma), 0), + -1, + ); + } + } + #[test] fn test_dict_pyobj_key() { install_test_hash_hook(); From d1d8d5265459919d21c5cb8fe396ab81b3dada4d Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 6 Aug 2026 05:14:30 +0900 Subject: [PATCH 07/10] jit, dict: re-validate a dict.lookup index against its key, and stamp the strategy id from the holder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `dict.lookup` fold emits the lookup and the value read as two residuals, and neither holds the dict's lock across the gap. `jit_dict_value_at` bounds-checked the index and nothing else, so a concurrent `del` between the two calls — which compacts the table, `IndexMap::shift_remove` closing the hole — left the index in range and naming a *different* live key. The answer was a wrong value, not a crash. `ll_dict_getitem_with_hash` has no such window: upstream runs the lookup and `d.entries[i].value` as one translated sequence. Read the value through `w_dict_unicode_value_at_checked`, which under one lock re-checks that the strategy still shares the `IndexMap` storage shape and that the entry at the index still holds the key the lookup was given — hash first, then the byte comparison under the callback-free bracket. Anything else answers null, and the caller's existing `GuardNonnull` side-exits to the generic residual. The re-validation also makes the optimizer's own CSE sound rather than merely bounded: `_optimize_call_dict_lookup` may delete a repeat of the same `(dict, key)` probe, so the index arriving at the value read can have been produced by an earlier iteration. `w_dict_strategy_id` was still deriving its stamp from the data half of `dstrategy.imp`. Every strategy singleton is a unit struct, so that word is the address of a zero-sized static and Rust does not guarantee two of them differ — two strategies could stamp the same id and a transition between them would be invisible to the iterators comparing it. Take the holder's own address: `DictStrategyRef` is not zero-sized and there is one per singleton. The unit test covers the case the bounds check cannot see — three keys, look up the second, delete the first, and read at the stale index, which is still in range and still holds a live entry. Removing the key re-validation fails it. Assisted-by: Claude --- pyre/pyre-jit-trace/src/helpers.rs | 22 +++-- .../src/jitcode_dispatch/specialize.rs | 9 +- pyre/pyre-object/src/dictmultiobject.rs | 95 ++++++++++++++++++- 3 files changed, 109 insertions(+), 17 deletions(-) diff --git a/pyre/pyre-jit-trace/src/helpers.rs b/pyre/pyre-jit-trace/src/helpers.rs index 22278259714..82363684045 100644 --- a/pyre/pyre-jit-trace/src/helpers.rs +++ b/pyre/pyre-jit-trace/src/helpers.rs @@ -144,23 +144,25 @@ pub extern "C" fn jit_dict_nth_value_versioned( /// Read a dict value at an entry index a traced `dict.lookup` settled on — /// `rordereddict.py:709 ll_dict_getitem`'s `d.entries[i].value` after -/// `ll_dict_lookup` returned the slot. The read goes through the live strategy +/// `ll_dict_lookup` returned the slot. The read goes through the live storage /// so a value-only overwrite stays visible, which is the whole reason the /// lookup and the value read are two operations rather than one. /// -/// The bounds check is load-bearing, not defensive. `_optimize_CALL_DICT_LOOKUP` -/// may delete a repeat of the same `(dict, key)` probe, so the index reaching -/// here can have been produced by an earlier iteration; an out-of-range index -/// answers `PY_NULL` and the caller's `GuardNonnull` side-exits to the generic -/// residual instead of indexing a compacted table. -pub extern "C" fn jit_dict_value_at(dict: i64, index: i64) -> i64 { +/// `w_dict_unicode_value_at_checked` re-validates the index against the key that +/// produced it; see there for why a bounds check alone is not enough. Anything +/// the index no longer describes answers `PY_NULL`, and the caller's +/// `GuardNonnull` side-exits to the generic residual. +pub extern "C" fn jit_dict_value_at(dict: i64, index: i64, key: i64, hash: i64) -> i64 { if index < 0 { return PY_NULL as i64; } - let dict = dict as PyObjectRef; unsafe { - pyre_object::dictmultiobject::w_dict_nth_item(dict, index as usize) - .map_or(PY_NULL as i64, |(_, value)| value as i64) + pyre_object::dictmultiobject::w_dict_unicode_value_at_checked( + dict as PyObjectRef, + index as usize, + key as PyObjectRef, + hash, + ) as i64 } } diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs index 95e2a5846a7..1552df8c7b1 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs @@ -5341,8 +5341,13 @@ pub(crate) fn try_walker_specialize_subscr( let value = ctx.trace_ctx.call_ref_typed_with_effect( crate::helpers::jit_dict_value_at as *const (), - &[list_op, index_op], - &[majit_ir::Type::Ref, majit_ir::Type::Int], + &[list_op, index_op, key_op, hash_op], + &[ + majit_ir::Type::Ref, + majit_ir::Type::Int, + majit_ir::Type::Ref, + majit_ir::Type::Int, + ], majit_ir::EffectInfo::new( majit_ir::ExtraEffect::CannotRaise, majit_ir::OopSpecIndex::None, diff --git a/pyre/pyre-object/src/dictmultiobject.rs b/pyre/pyre-object/src/dictmultiobject.rs index 7df5d55abd8..8012a32adb0 100644 --- a/pyre/pyre-object/src/dictmultiobject.rs +++ b/pyre/pyre-object/src/dictmultiobject.rs @@ -1974,11 +1974,15 @@ pub unsafe fn w_dict_strategy_id(obj: PyObjectRef) -> usize { return m.mstrategy as usize; } let d = &*(obj as *const W_DictObject); - // `&'static dyn DictStrategy` — the fat pointer carries both a - // vtable and a data pointer; the data pointer alone uniquely - // identifies the strategy singleton (`OBJECT_DICT_STRATEGY` etc.). - let raw: *const dyn crate::dictmultiobject::DictStrategy = d.dstrategy.imp; - raw as *const () as usize + // The holder's own address, not `imp`'s data pointer. Every strategy + // singleton is a unit struct, so the data half of the `&'static dyn + // DictStrategy` inside the holder is the address of a zero-sized static and + // Rust does not guarantee those differ between distinct strategies — two + // strategies could stamp the same id and a transition between them would be + // invisible to the iterators that compare this. `DictStrategyRef` is not + // zero-sized, so one holder per singleton gives one distinct address per + // strategy. + d.dstrategy as *const DictStrategyRef as usize } /// Key-set mutation state captured by dict iterators. @@ -4464,6 +4468,64 @@ pub unsafe fn w_dict_unicode_key_hash(w_key: *mut PyObject) -> i64 { .unwrap_or_else(|| crate::dict_eq_hook::missing_hash_hook()) } +/// `rordereddict.py:709 ll_dict_getitem` — the value half of the lookup, read +/// at the entry index [`w_dict_unicode_lookup_index`] settled on, and answering +/// null for anything the index no longer describes. +/// +/// The index is re-validated against the key that produced it, because the two +/// halves are two residuals and the lock is not held across the gap. Upstream +/// runs them as one translated sequence under the GIL and can index straight +/// in; here another thread's `del` compacts the table (`IndexMap::shift_remove` +/// closes the hole), so an index that was in range and correct at lookup time +/// can name a *different* live key by the time it is read. A bounds check does +/// not see that — the index is still in range — and the answer would be a wrong +/// value rather than a crash. Re-validating also makes the optimizer's own CSE +/// safe: `_optimize_call_dict_lookup` may delete a repeat of the same +/// `(dict, key)` probe, so the index arriving here can have been produced by an +/// earlier iteration. +/// +/// The strategy is re-checked under the lock for the same reason: the trace +/// guarded it before the call, and only the Unicode and Object strategies share +/// this `IndexMap` storage shape. +/// +/// A null answer fails the caller's `GuardNonnull` and side-exits to the generic +/// residual, which redoes the whole subscript. +/// +/// # Safety +/// `obj` must be a live regular `W_DictObject` (not a module dict), and `hash` +/// the digest `object_key_for` produces for `w_key`. +pub unsafe fn w_dict_unicode_value_at_checked( + obj: PyObjectRef, + index: usize, + w_key: PyObjectRef, + hash: i64, +) -> PyObjectRef { + lock_dict_refs!(_dict_guard, obj, w_key); + if !matches!( + w_dict_get_strategy(obj).strategy_kind(), + StrategyKind::Unicode | StrategyKind::Object + ) { + return std::ptr::null_mut(); + } + let dict = &*(obj as *const W_DictObject); + let entries = &*(dict.dstorage as *const indexmap::IndexMap); + let Some((entry_key, &value)) = entries.get_index(index) else { + return std::ptr::null_mut(); + }; + if entry_key.hash != hash { + return std::ptr::null_mut(); + } + crate::dict_eq_hook::take_eq_error(); + crate::dict_eq_hook::begin_callback_free_probe(); + let same_key = dict_keys_equal(entry_key.obj, w_key); + let probe_needed_user_code = crate::dict_eq_hook::end_callback_free_probe(); + if same_key && !probe_needed_user_code { + value + } else { + std::ptr::null_mut() + } +} + /// Internal helper: `ModuleDictStrategy::items` body for pyre's /// W_ModuleDictObject — branches on `is_object_strategy` and emits /// from whichever storage half is live. Called only from the @@ -6647,6 +6709,29 @@ mod tests { w_dict_unicode_lookup_index(dict, gamma, w_dict_unicode_key_hash(gamma), 0), -1, ); + + // The value read answers for the key that produced the index, not + // for whatever now sits at it. + w_dict_setitem_str(dict, "gamma", w_int_new(3)); + let beta_hash = w_dict_unicode_key_hash(beta); + let beta_value = w_dict_getitem_str(dict, "beta").unwrap(); + assert_eq!(w_dict_unicode_lookup_index(dict, beta, beta_hash, 0), 1); + assert_eq!( + w_dict_unicode_value_at_checked(dict, 1, beta, beta_hash), + beta_value, + ); + + // Deleting an earlier key compacts the table, so index 1 now names + // "gamma". It is still IN RANGE and still holds a live entry, so a + // bounds check passes and would answer gamma's value for beta's + // index — only the key re-validation catches it. + w_dict_delitem_str(dict, "alpha"); + assert_eq!(w_dict_unicode_lookup_index(dict, beta, beta_hash, 0), 0); + assert!(w_dict_unicode_value_at_checked(dict, 1, beta, beta_hash).is_null()); + assert_eq!( + w_dict_unicode_value_at_checked(dict, 0, beta, beta_hash), + beta_value, + ); } } From bacbd2737c0c9fbaac5a74719a7be20c43462d6f Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 6 Aug 2026 05:52:30 +0900 Subject: [PATCH 08/10] bench: re-record the jitstats baselines the rebase blended MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebasing onto origin/main conflicted on 44 `.jitstats` files, every one the same shape: the base had begun recording `fbw_rolled_back_with_effects` while this branch had moved `guard_failures`. The conflicts were resolved key-wise — the union, taking this branch's value where both sides named the same counter — but those values were measured on the *pre-rebase* base, so they were provisional. These are the measurements on the rebased tree. Every value here was confirmed against CI, which ran the same commit against the blended baselines and reported its own observations: * the 7 dynasm and 8 cranelift moves match the macos-latest job exactly (3610, 606, 817, 607, 1670, 1202, and 463/656 for the two backends); * the 10 wasm moves match the ubuntu-24.04 job exactly, including every `bridges_compiled` — that is the only CI job that runs the wasm backend. `pickle_ctor_args.cranelift` is the one value CI does not corroborate, because the ratio gate returns before `_apply_snapshot_gate` and that bench fails the ratio gate on CI — on origin/main too, at 44.8x and 62.3x against a 36x ceiling, so its counters are never reached there. 436 -> 201 is what reproduces here across two independently built binaries. No badness counter moved anywhere in this set: `loops_aborted`, `internal_compile_panics`, the three `descr_set_*` and `fbw_rolled_back_with_effects` are unchanged. With these recorded, `check.py --backend dynasm` and `--backend cranelift` are 386/386 locally. Assisted-by: Claude --- pyre/bench/synth/closure_per_call.wasm.jitstats | 2 +- .../synth/comprehension_object_append_hot.cranelift.jitstats | 2 +- .../synth/comprehension_object_append_hot.dynasm.jitstats | 2 +- .../synth/exception_catching_frame_tb_node.wasm.jitstats | 4 ++-- .../exception_inline_callee_tb_frames.cranelift.jitstats | 2 +- .../synth/exception_inline_callee_tb_frames.dynasm.jitstats | 2 +- .../synth/exception_inline_callee_tb_frames.wasm.jitstats | 4 ++-- .../exception_reentry_guard_finally_residual.wasm.jitstats | 2 +- .../synth/exception_traceback_frame_lineno.cranelift.jitstats | 2 +- .../synth/exception_traceback_frame_lineno.dynasm.jitstats | 2 +- .../synth/exception_traceback_frame_lineno.wasm.jitstats | 2 +- .../synth/exception_traceback_lineno_chain.cranelift.jitstats | 2 +- .../synth/exception_traceback_lineno_chain.dynasm.jitstats | 2 +- .../synth/exception_traceback_lineno_chain.wasm.jitstats | 4 ++-- .../gc_bug_bridge_flavor_traceback_names.cranelift.jitstats | 2 +- .../gc_bug_bridge_flavor_traceback_names.dynasm.jitstats | 2 +- .../synth/gc_bug_bridge_flavor_traceback_names.wasm.jitstats | 4 ++-- .../synth/nested_list_comprehension_hot.cranelift.jitstats | 2 +- .../bench/synth/nested_list_comprehension_hot.dynasm.jitstats | 2 +- pyre/bench/synth/pickle_ctor_args.cranelift.jitstats | 2 +- .../synth/pickle_terminal_raise_resume.cranelift.jitstats | 2 +- pyre/bench/synth/pickle_terminal_raise_resume.dynasm.jitstats | 2 +- pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats | 2 +- .../bench/synth/recursive_call_frame_relocation.wasm.jitstats | 2 +- pyre/bench/synth/str_fstring.wasm.jitstats | 2 +- 25 files changed, 29 insertions(+), 29 deletions(-) diff --git a/pyre/bench/synth/closure_per_call.wasm.jitstats b/pyre/bench/synth/closure_per_call.wasm.jitstats index a75a9bc1d92..fb6a485e39d 100644 --- a/pyre/bench/synth/closure_per_call.wasm.jitstats +++ b/pyre/bench/synth/closure_per_call.wasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=471 +guard_failures=470 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/comprehension_object_append_hot.cranelift.jitstats b/pyre/bench/synth/comprehension_object_append_hot.cranelift.jitstats index 86d936cfe36..162430619de 100644 --- a/pyre/bench/synth/comprehension_object_append_hot.cranelift.jitstats +++ b/pyre/bench/synth/comprehension_object_append_hot.cranelift.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=3612 +guard_failures=3610 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/comprehension_object_append_hot.dynasm.jitstats b/pyre/bench/synth/comprehension_object_append_hot.dynasm.jitstats index 86d936cfe36..162430619de 100644 --- a/pyre/bench/synth/comprehension_object_append_hot.dynasm.jitstats +++ b/pyre/bench/synth/comprehension_object_append_hot.dynasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=3612 +guard_failures=3610 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/exception_catching_frame_tb_node.wasm.jitstats b/pyre/bench/synth/exception_catching_frame_tb_node.wasm.jitstats index b1156f9b9d9..3ff10b319d6 100644 --- a/pyre/bench/synth/exception_catching_frame_tb_node.wasm.jitstats +++ b/pyre/bench/synth/exception_catching_frame_tb_node.wasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=2 +bridges_compiled=3 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=401 +guard_failures=601 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/exception_inline_callee_tb_frames.cranelift.jitstats b/pyre/bench/synth/exception_inline_callee_tb_frames.cranelift.jitstats index 4ceb46593e9..a4ad9babaf5 100644 --- a/pyre/bench/synth/exception_inline_callee_tb_frames.cranelift.jitstats +++ b/pyre/bench/synth/exception_inline_callee_tb_frames.cranelift.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=603 +guard_failures=606 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/exception_inline_callee_tb_frames.dynasm.jitstats b/pyre/bench/synth/exception_inline_callee_tb_frames.dynasm.jitstats index 4ceb46593e9..a4ad9babaf5 100644 --- a/pyre/bench/synth/exception_inline_callee_tb_frames.dynasm.jitstats +++ b/pyre/bench/synth/exception_inline_callee_tb_frames.dynasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=603 +guard_failures=606 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/exception_inline_callee_tb_frames.wasm.jitstats b/pyre/bench/synth/exception_inline_callee_tb_frames.wasm.jitstats index cfdee3cf187..8aeebdb89ea 100644 --- a/pyre/bench/synth/exception_inline_callee_tb_frames.wasm.jitstats +++ b/pyre/bench/synth/exception_inline_callee_tb_frames.wasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=3 +bridges_compiled=5 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1685 +guard_failures=1008 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/exception_reentry_guard_finally_residual.wasm.jitstats b/pyre/bench/synth/exception_reentry_guard_finally_residual.wasm.jitstats index a43dc131d42..97cc4a44a37 100644 --- a/pyre/bench/synth/exception_reentry_guard_finally_residual.wasm.jitstats +++ b/pyre/bench/synth/exception_reentry_guard_finally_residual.wasm.jitstats @@ -1,4 +1,4 @@ -bridges_compiled=11 +bridges_compiled=12 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 diff --git a/pyre/bench/synth/exception_traceback_frame_lineno.cranelift.jitstats b/pyre/bench/synth/exception_traceback_frame_lineno.cranelift.jitstats index 38e4fbc5545..2c38c265ac4 100644 --- a/pyre/bench/synth/exception_traceback_frame_lineno.cranelift.jitstats +++ b/pyre/bench/synth/exception_traceback_frame_lineno.cranelift.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=812 +guard_failures=817 internal_compile_panics=0 loops_aborted=0 loops_compiled=17 diff --git a/pyre/bench/synth/exception_traceback_frame_lineno.dynasm.jitstats b/pyre/bench/synth/exception_traceback_frame_lineno.dynasm.jitstats index 38e4fbc5545..2c38c265ac4 100644 --- a/pyre/bench/synth/exception_traceback_frame_lineno.dynasm.jitstats +++ b/pyre/bench/synth/exception_traceback_frame_lineno.dynasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=812 +guard_failures=817 internal_compile_panics=0 loops_aborted=0 loops_compiled=17 diff --git a/pyre/bench/synth/exception_traceback_frame_lineno.wasm.jitstats b/pyre/bench/synth/exception_traceback_frame_lineno.wasm.jitstats index eff716cf5be..f1d4098f127 100644 --- a/pyre/bench/synth/exception_traceback_frame_lineno.wasm.jitstats +++ b/pyre/bench/synth/exception_traceback_frame_lineno.wasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=815 +guard_failures=820 internal_compile_panics=0 loops_aborted=0 loops_compiled=17 diff --git a/pyre/bench/synth/exception_traceback_lineno_chain.cranelift.jitstats b/pyre/bench/synth/exception_traceback_lineno_chain.cranelift.jitstats index b4b631c02bf..c94f754ae93 100644 --- a/pyre/bench/synth/exception_traceback_lineno_chain.cranelift.jitstats +++ b/pyre/bench/synth/exception_traceback_lineno_chain.cranelift.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=605 +guard_failures=607 internal_compile_panics=0 loops_aborted=0 loops_compiled=5 diff --git a/pyre/bench/synth/exception_traceback_lineno_chain.dynasm.jitstats b/pyre/bench/synth/exception_traceback_lineno_chain.dynasm.jitstats index b4b631c02bf..c94f754ae93 100644 --- a/pyre/bench/synth/exception_traceback_lineno_chain.dynasm.jitstats +++ b/pyre/bench/synth/exception_traceback_lineno_chain.dynasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=605 +guard_failures=607 internal_compile_panics=0 loops_aborted=0 loops_compiled=5 diff --git a/pyre/bench/synth/exception_traceback_lineno_chain.wasm.jitstats b/pyre/bench/synth/exception_traceback_lineno_chain.wasm.jitstats index c98a602a429..43bb687d2a9 100644 --- a/pyre/bench/synth/exception_traceback_lineno_chain.wasm.jitstats +++ b/pyre/bench/synth/exception_traceback_lineno_chain.wasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=2 +bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=404 +guard_failures=804 internal_compile_panics=0 loops_aborted=0 loops_compiled=5 diff --git a/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.cranelift.jitstats b/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.cranelift.jitstats index 66923ce27ec..0f6bbf28a78 100644 --- a/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.cranelift.jitstats +++ b/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.cranelift.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1671 +guard_failures=1670 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.dynasm.jitstats b/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.dynasm.jitstats index 66923ce27ec..0f6bbf28a78 100644 --- a/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.dynasm.jitstats +++ b/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.dynasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1671 +guard_failures=1670 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.wasm.jitstats b/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.wasm.jitstats index c9c01e5d40b..c229b7a7832 100644 --- a/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.wasm.jitstats +++ b/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.wasm.jitstats @@ -1,9 +1,9 @@ -bridges_compiled=8 +bridges_compiled=9 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=2036 +guard_failures=2037 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/nested_list_comprehension_hot.cranelift.jitstats b/pyre/bench/synth/nested_list_comprehension_hot.cranelift.jitstats index ee7d2607be7..c98bf212154 100644 --- a/pyre/bench/synth/nested_list_comprehension_hot.cranelift.jitstats +++ b/pyre/bench/synth/nested_list_comprehension_hot.cranelift.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1204 +guard_failures=1202 internal_compile_panics=0 loops_aborted=0 loops_compiled=2 diff --git a/pyre/bench/synth/nested_list_comprehension_hot.dynasm.jitstats b/pyre/bench/synth/nested_list_comprehension_hot.dynasm.jitstats index ee7d2607be7..c98bf212154 100644 --- a/pyre/bench/synth/nested_list_comprehension_hot.dynasm.jitstats +++ b/pyre/bench/synth/nested_list_comprehension_hot.dynasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=1204 +guard_failures=1202 internal_compile_panics=0 loops_aborted=0 loops_compiled=2 diff --git a/pyre/bench/synth/pickle_ctor_args.cranelift.jitstats b/pyre/bench/synth/pickle_ctor_args.cranelift.jitstats index 7531629e55f..a2a8fa3ce59 100644 --- a/pyre/bench/synth/pickle_ctor_args.cranelift.jitstats +++ b/pyre/bench/synth/pickle_ctor_args.cranelift.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=436 +guard_failures=201 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/pickle_terminal_raise_resume.cranelift.jitstats b/pyre/bench/synth/pickle_terminal_raise_resume.cranelift.jitstats index 4ce9dd1e102..3e20b65303d 100644 --- a/pyre/bench/synth/pickle_terminal_raise_resume.cranelift.jitstats +++ b/pyre/bench/synth/pickle_terminal_raise_resume.cranelift.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=668 +guard_failures=656 internal_compile_panics=0 loops_aborted=1 loops_compiled=36 diff --git a/pyre/bench/synth/pickle_terminal_raise_resume.dynasm.jitstats b/pyre/bench/synth/pickle_terminal_raise_resume.dynasm.jitstats index f39e9177c47..3add0c6956f 100644 --- a/pyre/bench/synth/pickle_terminal_raise_resume.dynasm.jitstats +++ b/pyre/bench/synth/pickle_terminal_raise_resume.dynasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=482 +guard_failures=463 internal_compile_panics=0 loops_aborted=1 loops_compiled=36 diff --git a/pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats b/pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats index e6d2a5d24d9..756a0c8ba33 100644 --- a/pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats +++ b/pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=5 -guard_failures=483 +guard_failures=464 internal_compile_panics=0 loops_aborted=13 loops_compiled=73 diff --git a/pyre/bench/synth/recursive_call_frame_relocation.wasm.jitstats b/pyre/bench/synth/recursive_call_frame_relocation.wasm.jitstats index fa8c6b47e71..d98f1879e71 100644 --- a/pyre/bench/synth/recursive_call_frame_relocation.wasm.jitstats +++ b/pyre/bench/synth/recursive_call_frame_relocation.wasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=650 +guard_failures=649 internal_compile_panics=0 loops_aborted=0 loops_compiled=3 diff --git a/pyre/bench/synth/str_fstring.wasm.jitstats b/pyre/bench/synth/str_fstring.wasm.jitstats index a9521e7fa2e..3fb932f4dc9 100644 --- a/pyre/bench/synth/str_fstring.wasm.jitstats +++ b/pyre/bench/synth/str_fstring.wasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_rolled_back_with_effects=0 -guard_failures=667 +guard_failures=666 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 From dcc7bef828969c17f793fd947add3dc4662d62c1 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 6 Aug 2026 05:52:39 +0900 Subject: [PATCH 09/10] bench: record the missing wasm jitstats baseline for exception_escape_hot_callee_tb_node_once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fixture arrived with dynasm and cranelift baselines and no wasm one, so the wasm gate reported "no committed jit-stats baseline" rather than comparing anything. That is a failure by construction — check.py treats an absent baseline as a way for the gate to be silently disarmed, not as a pass — and origin/main is red on it at the same sha for the same reason. Unlike the rest of this branch's baselines, this value is not corroborated by CI: CI only reports that the file is missing, and its log does not print the raw counters. What supports it is that every other wasm baseline recorded on this host matched the ubuntu-24.04 job's observation exactly, all ten of them including each `bridges_compiled`. Kept as its own commit: it closes a gap that predates this branch, so it can be taken or dropped independently of the fold work. Assisted-by: Claude --- ...xception_escape_hot_callee_tb_node_once.wasm.jitstats | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 pyre/bench/synth/exception_escape_hot_callee_tb_node_once.wasm.jitstats diff --git a/pyre/bench/synth/exception_escape_hot_callee_tb_node_once.wasm.jitstats b/pyre/bench/synth/exception_escape_hot_callee_tb_node_once.wasm.jitstats new file mode 100644 index 00000000000..62e3f6d3e6a --- /dev/null +++ b/pyre/bench/synth/exception_escape_hot_callee_tb_node_once.wasm.jitstats @@ -0,0 +1,9 @@ +bridges_compiled=5 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_rolled_back_with_effects=0 +guard_failures=1016 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=15 From a39b76c59417f7004689c7dcf56dd29e5a05cc54 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 6 Aug 2026 06:12:34 +0900 Subject: [PATCH 10/10] dict: say that only FLAG_LOOKUP is implemented, rather than that every flag is the same MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The doc claimed an `IndexMap` "keeps no free-slot bookkeeping, so every flag answers the same index". That reads as a general `ll_dict_lookup` port and it is not one. On a miss `FLAG_STORE` answers the free slot the caller is to write into and `FLAG_DELETE` walks to the tombstone — both read exactly the free-slot bookkeeping that is absent here — while this helper answers -1 for every miss. A store or delete lookup would be wrong, not conservative. Nothing emits either: the recorder only ever passes 0, and the optimizer requires `arg(4)` to be a constant in {0,1}, which is why the parameter exists at all. Name it and pin it with a debug assertion, so a future producer that passes 1 stops here instead of silently reading -1 as "absent". `debug_assert` is compiled out of the release profile, which does not enable debug assertions, so the shipped code is unchanged. Assisted-by: Claude --- pyre/pyre-object/src/dictmultiobject.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pyre/pyre-object/src/dictmultiobject.rs b/pyre/pyre-object/src/dictmultiobject.rs index 8012a32adb0..384f3e5250f 100644 --- a/pyre/pyre-object/src/dictmultiobject.rs +++ b/pyre/pyre-object/src/dictmultiobject.rs @@ -4403,9 +4403,17 @@ pub unsafe fn w_dict_nth_item_object_strategy( /// hash the caller already computed. /// /// `flag` is the `FLAG_LOOKUP` / `FLAG_STORE` / `FLAG_DELETE` selector -/// `ll_dict_lookup` takes (`rordereddict.py:1041-1060`). An `IndexMap` keeps no -/// free-slot bookkeeping, so every flag answers the same index; the argument -/// exists because the optimizer reads it off `arg(4)` (`heap.py:497-500`). +/// `ll_dict_lookup` takes (`rordereddict.py:1041-1060`), and **only +/// `FLAG_LOOKUP` is implemented here**. The argument exists because the +/// optimizer reads it off `arg(4)` (`heap.py:497-500`) and requires it to be a +/// constant; the recorder only ever emits 0. +/// +/// The other two are not "the same answer" — they are not expressible. On a +/// miss `FLAG_STORE` answers the free slot the caller is to write into, and +/// `FLAG_DELETE` walks to the tombstone, both of which read the free-slot +/// bookkeeping an `IndexMap` does not keep. This helper answers -1 for every +/// miss, so a store or delete lookup would be wrong rather than merely +/// conservative. /// /// Only a Unicode-strategy dict reaches here, so every stored key is an exact /// `str` and the probe's comparisons are WTF-8 byte equality. The @@ -4436,8 +4444,9 @@ pub unsafe fn w_dict_unicode_lookup_index( w_dict: *mut PyObject, w_key: *mut PyObject, hash: i64, - _flag: i64, + flag: i64, ) -> i64 { + debug_assert_eq!(flag, 0, "only FLAG_LOOKUP is implemented"); lock_dict_refs!(_dict_guard, w_dict, w_key); let dict = &*(w_dict as *const W_DictObject); let entries = &*(dict.dstorage as *const indexmap::IndexMap);