Skip to content

jit: restore the raise-bearing method-form inline, and inline a user binop/compare dunder that reads self.attr #954

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 2 commits into from
Aug 1, 2026
Merged

jit: restore the raise-bearing method-form inline, and inline a user binop/compare dunder that reads self.attr #954

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
351d1cd
jit: key the raise decline on widened_method_form, not allow_method_l…
youknowone Aug 1, 2026
79ed4a2
jit: inline a user binop/compare dunder whose body reads self.attr
youknowone Aug 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 31 additions & 41 deletions pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,11 +415,11 @@ pub(crate) fn callee_body_contains_raise(body_code: &[u8]) -> bool {

/// Whether a method-form callee body is free of `LoadAttr` residuals.
///
/// Consulted only by the entries that pass `allow_method_load_attr = false`.
/// A `self.attr` read in the body is what makes it answer `false`, which is the
/// common shape (`def at(self, i): return self.v + i`), so an entry that opts
/// out of the check trades a narrower inline surface for the ability to inline
/// ordinary accessor methods.
/// A `self.attr` read in the body is what makes it answer `false`, and that is
/// the common shape (`def at(self, i): return self.v + i`). No entry declines
/// on it any more; it names the bodies that reach the inline only through the
/// widened surface, which the two declines in
/// `try_walker_inline_resolved_user_call` are scoped to.
pub(crate) fn method_form_callee_body_supported(
body_code: &[u8],
callee_descr_refs: &[DescrRef],
Expand Down Expand Up @@ -1749,7 +1749,6 @@ pub(crate) fn try_walker_inline_user_call<Sym: WalkSym>(
has_closure,
None,
None,
true,
false,
None,
)
Expand Down Expand Up @@ -2477,7 +2476,6 @@ pub(crate) fn try_walker_inline_resolved_user_call<Sym: WalkSym>(
has_closure: bool,
exception_receiver_guard: Option<ExceptionInlineReceiverGuard>,
arg_class_guard: Option<ArgClassGuard>,
allow_method_load_attr: bool,
require_str_result: bool,
constructor_result: Option<(OpRef, ConcreteValue)>,
) -> Result<Option<(DispatchOutcome, usize)>, DispatchError> {
Expand Down Expand Up @@ -2724,11 +2722,10 @@ pub(crate) fn try_walker_inline_resolved_user_call<Sym: WalkSym>(
if !bridge_rec_root_selfrec && fbw_hazardous_inline_denied(callee_code_key) {
return Ok(None);
}
// True when only the widened method-form surface reaches this callee: an
// unbound `self.attr` accessor body, which the narrow surface declines.
// An unbound method-form callee whose body reads `self.attr`. Every entry
// inlines one; the two declines below are what that reach costs.
let widened_method_form = method_form
&& bound_method.is_none()
&& allow_method_load_attr
&& !method_form_callee_body_supported(body.code, callee_descr_refs);
// A legacy, unseeded inline sub-walk inside a FOR_ITER body resumes a guard
// at the caller's CALL boundary, so deopt re-executes the whole callee.
Expand Down Expand Up @@ -2767,12 +2764,12 @@ pub(crate) fn try_walker_inline_resolved_user_call<Sym: WalkSym>(
// body is still admitted from there — it has nothing that can
// abort.
//
// The widened method-form surface stays out: its deferred call
// is the `self.attr` read's own dispatch, which the lever
// resolves to a builtin rather than a body it can inline, so the
// admission spends the abort and then denies the callee anyway.
// Declining here reaches the same residual call without retiring
// the enclosing loop.
// A body that reads `self.attr` stays out as well. Admitting
// one costs an abort that retires the enclosing loop before the
// deny takes effect -- `synth/type_metatype_method_call` went
// `loops_aborted` 0 -> 1 and `bridges_compiled` 47 -> 44 on the
// admission alone. Declining here reaches the same residual
// call with the loop intact.
foriter_deferred_admit = arg_class_guard.is_none()
&& !widened_method_form
&& !fbw_foriter_deferred_call_denied(callee_code_key);
Expand Down Expand Up @@ -2808,26 +2805,25 @@ pub(crate) fn try_walker_inline_resolved_user_call<Sym: WalkSym>(
}
}
if method_form && bound_method.is_none() {
// Two surfaces for an unbound method-form callee. The narrow one
// declines any `self.attr` read in the body. The wide one admits it,
// and pays for the reach with a body that raises: the sub-walk records
// into the handler region, and a guard whose resume coordinate lands on
// the `Reraise` needs ref registers the recorded path never wrote
// (`collect_callee_active_boxes`). That decline arrives mid-recording
// on a non-effect-free opcode, so it has no mid-body carrier and the
// whole enclosing loop is discarded. Decline such a body here instead,
// where the call simply stays residual and the loop still compiles.
let declined = if allow_method_load_attr {
callee_body_contains_raise(body.code)
} else {
!method_form_callee_body_supported(body.code, callee_descr_refs)
};
if declined {
// The narrow surface declines any `self.attr` read in the body.
//
// The wide one admits it, and pays for the reach with a body that also
// raises: the sub-walk records into the handler region, and a guard
// whose resume coordinate lands on the `Reraise` needs ref registers
// the recorded path never wrote (`collect_callee_active_boxes`). That
// decline arrives mid-recording on a non-effect-free opcode, so it has
// no mid-body carrier and the whole enclosing loop is discarded.
// Decline here instead, where the call stays residual and the loop
// still compiles.
//
// Both conjuncts are load-bearing. Without `widened_method_form` this
// also withdraws a raise-bearing body that reads no attribute, which
// every entry inlined before the widening -- 3.6x on
// `for i in range(400000): t += b.bump(i)` over
// `def bump(self, n): if n < 0: raise ValueError(n); return n + 1`.
if widened_method_form && callee_body_contains_raise(body.code) {
if std::env::var_os("PYRE_FBW_INLINE_DIAG").is_some() {
eprintln!(
"[inline-method-form] decline pc={} allow_load_attr={allow_method_load_attr}",
op.pc
);
eprintln!("[inline-method-form] decline pc={}", op.pc);
}
return Ok(None);
}
Expand Down Expand Up @@ -4530,7 +4526,6 @@ pub(crate) fn try_walker_inline_type_call<Sym: WalkSym>(
// `__init__` bodies are `self.x = ...` stores; the sub-walk folds them
// to slot writes on the fresh instance exactly as the property-setter
// route folds its own.
true,
false,
Some((instance, ConcreteValue::Ref(concrete_instance))),
)?;
Expand Down Expand Up @@ -4678,7 +4673,6 @@ pub(crate) fn try_walker_inline_exception_string_override<Sym: WalkSym>(
Some((r_args[2], concrete_receiver, w_class, version_tag)),
None,
true,
true,
None,
)?
else {
Expand Down Expand Up @@ -4800,7 +4794,6 @@ pub(crate) fn try_walker_inline_property_get<Sym: WalkSym>(
// Getter bodies commonly read `self._slot` — a LOAD_ATTR the method-form
// support gate would otherwise reject; the sub-walk folds it to a slot
// read (same allowance the exception `__str__`/`__repr__` override uses).
true,
false,
None,
)
Expand Down Expand Up @@ -4899,7 +4892,6 @@ pub(crate) fn try_walker_inline_property_set<Sym: WalkSym>(
has_closure,
Some((obj, concrete_obj, w_type, version_tag)),
None,
true,
false,
None,
)
Expand Down Expand Up @@ -5056,7 +5048,6 @@ pub(crate) fn try_walker_inline_user_binop<Sym: WalkSym>(
Some((lhs, concrete_lhs, w_class, version_tag)),
Some((rhs, concrete_rhs, w_typ_r.as_ptr())),
false,
Comment on lines 5048 to 5050

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard the RHS type version before widening dunder inlining

With the method-attribute gate removed here (and in the compare twin), a dunder such as A.__add__(self, other): return self.x is now inlined, but the reflected-method decision was made using the RHS subtype relationship while arg_class_guard only pins the RHS object's physical type and w_class (lines 3072-3104), not that class's version tag. If a compatible RHS class changes __bases__ after the trace becomes hot, its objects retain the same w_class, so the trace keeps calling A.__add__ even when the interpreter would now prioritize the RHS subclass's __radd__; the same issue affects rich comparisons. Pin the RHS type version/MRO dependency before admitting these newly widened bodies.

Useful? React with 👍 / 👎.

false,
None,
)?
else {
Expand Down Expand Up @@ -5202,7 +5193,6 @@ pub(crate) fn try_walker_inline_user_compareop<Sym: WalkSym>(
Some((lhs, concrete_lhs, w_class, version_tag)),
Some((rhs, concrete_rhs, w_typ_r.as_ptr())),
false,
false,
None,
)?
else {
Expand Down
Loading