-
Notifications
You must be signed in to change notification settings - Fork 19
jit: resolve a walk's coordinates from the frame and operand offset that produced them; gate the vendored CPython suite #1111
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.
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
Changes from all commits
bc189d2
a07a4d4
c16eeae
d7eb409
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| # Locals of an inlined callee must survive a guard that fails inside the | ||
| # callee's own body. A callee's `LOAD_FAST`/`STORE_FAST` lower to | ||
| # `getarrayitem_vable_*`/`setarrayitem_vable_*` on its own frame array, and the | ||
| # split between the `locals_cells_stack_w` prefix and the operand-stack region | ||
| # is that frame's own local count. Both shapes are covered: a callee with MORE | ||
| # locals than its caller (a `STORE_FAST` mistaken for an operand-stack push | ||
| # folds away, and the resumed frame reads the slot back as NULL) and one with | ||
| # FEWER (an operand-stack cell mistaken for a local). | ||
| # | ||
| # The guard is an attribute read off a receiver whose class alternates between | ||
| # rounds, so every round after the first resumes inside the callee. | ||
|
|
||
| N = 400 | ||
| ROUNDS = 60 | ||
|
|
||
|
|
||
| class Shape: | ||
| def __init__(self, v): | ||
| self.v = v | ||
|
|
||
|
|
||
| class Other: | ||
| def __init__(self, v): | ||
| self.v = v | ||
|
|
||
|
|
||
| def sink(a, b, c): | ||
| return (a * 7 + b * 3 + c) % 1000003 | ||
|
|
||
|
|
||
| def wide_callee(o, k): | ||
| # Six locals — more than `narrow_driver` has. `t0`/`t2` are stored before | ||
| # the guarded `o.v` read and consumed after it. | ||
| t0 = k * 2 + 1 | ||
| t1 = t0 + 5 | ||
| t2 = t1 * 3 | ||
| v = o.v | ||
| return sink(t0, t2, v) | ||
|
|
||
|
|
||
| def narrow_driver(n, o): | ||
| acc = 0 | ||
| for i in range(n): | ||
| acc = (acc + wide_callee(o, i)) % 1000003 | ||
| return acc | ||
|
|
||
|
|
||
| def narrow_callee(o): | ||
| # Two locals — fewer than `wide_driver` has. | ||
| v = o.v | ||
| return (v, v + 1, v + 2) | ||
|
|
||
|
|
||
| def wide_driver(n, o): | ||
| a = 0 | ||
| b = 1 | ||
| c = 2 | ||
| d = 3 | ||
| e = 4 | ||
| f = 5 | ||
| g = 6 | ||
| acc = 0 | ||
| for i in range(n): | ||
| x, y, z = narrow_callee(o) | ||
| acc = (acc + x + y + z + a + b + c + d + e + f + g + i) % 1000003 | ||
| return acc | ||
|
|
||
|
|
||
| # sum over i in 0..N-1 of (32 * i + 72), taken mod 1000003 | ||
| NARROW_EXPECTED = (32 * (N * (N - 1) // 2) + 72 * N) % 1000003 | ||
| # per iteration: (11 + 12 + 13) + (0 + 1 + ... + 6) + i | ||
| WIDE_EXPECTED = ((36 + 21) * N + N * (N - 1) // 2) % 1000003 | ||
|
|
||
| warm = Shape(11) | ||
| flip = Other(11) | ||
|
|
||
| for round_ in range(ROUNDS): | ||
| for receiver in (warm, flip): | ||
| assert narrow_driver(N, receiver) == NARROW_EXPECTED | ||
| assert wide_driver(N, receiver) == WIDE_EXPECTED | ||
|
|
||
| print("OK") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| # The line number a frame reports while a residual runs inside a call chain | ||
| # that was inlined more than one level deep. | ||
| # | ||
| # A residual executed inside an inlined callee temporarily publishes | ||
| # `last_instr` onto the outer traced frame, so that a frame reader running | ||
| # during the call (`sys._getframe().f_lineno`, a warning's registry key, a | ||
| # traceback) sees the line the call was made from rather than whatever the last | ||
| # resume point left behind. That coordinate is derived from the CALL | ||
| # instruction's JitCode offset, which indexes the outer frame's JitCode only | ||
| # while the walk is that frame's own. One level down, the offset belongs to the | ||
| # intermediate callee, and mapping it through the outer frame's pc tables names | ||
| # whatever line that byte happens to land on — here the `def` line's body start | ||
| # instead of the call. | ||
| # | ||
| # `driver` runs the traced loop, inlines `mid`, which inlines `leaf`; the | ||
| # `sys._getframe` residual inside `leaf` is what publishes the coordinate. The | ||
| # loop collects every line `driver` reports across the run, so a single wrong | ||
| # iteration is caught: the set must hold exactly the one call line. | ||
|
|
||
| import sys | ||
|
|
||
| N = 3000 | ||
|
|
||
|
|
||
| def leaf(k): | ||
| # Frame depths: 0 = leaf, 1 = mid, 2 = driver. | ||
| return sys._getframe(2).f_lineno | ||
|
|
||
|
|
||
| def mid(k): | ||
| return leaf(k) | ||
|
|
||
|
|
||
| def driver(n): | ||
| seen = set() | ||
| i = 0 | ||
| while i < n: | ||
| seen.add(mid(i)) # <-- the only line `driver` may ever report | ||
| i += 1 | ||
| return sorted(seen) | ||
|
|
||
|
|
||
| CALL_LINE = driver.__code__.co_firstlineno + 4 | ||
| observed = driver(N) | ||
| assert observed == [CALL_LINE], (CALL_LINE, observed) | ||
|
|
||
| print("OK") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -147,20 +147,39 @@ pub(crate) fn inflight_foriter_body_pc(body: InflightForiterBody) -> Option<usiz | |
| match body { | ||
| InflightForiterBody::Py(body_pc) => Some(body_pc), | ||
| InflightForiterBody::Jit { | ||
| outer_jitcode_index, | ||
| jitcode_index, | ||
| op_pc, | ||
| } => crate::state::pyjitcode_for_jitcode_index(outer_jitcode_index as i32).map(|jc| { | ||
| } => crate::state::pyjitcode_for_jitcode_index(jitcode_index).map(|jc| { | ||
| crate::py_coord::containing_py_pc_for_jitcode_pc(&jc.metadata, op_pc) as usize + 1 | ||
|
Comment on lines
+152
to
153
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an inlined callee's AGENTS.md reference: AGENTS.md:L32-L42 Useful? React with 👍 / 👎. |
||
| }), | ||
| } | ||
| } | ||
|
|
||
| /// Capture the native coordinates that identify a `for_iter_next` residual. | ||
| /// The Python continue-arm fallthrough is intentionally not derived here. | ||
| /// | ||
| /// `op_pc` is an offset into the JitCode the walk is currently executing, so | ||
| /// the identity paired with it must be that JitCode's. Inside an inline | ||
| /// sub-walk that is the callee's ([`InlineCalleeConsts::jitcode_index`], the | ||
| /// same resolution `build_multi_frame_miframe` applies to its innermost | ||
| /// frame); `fbw_mode.snapshot_sym` still names the outer portal. Pairing the | ||
| /// portal with a callee offset invents a coordinate: `inflight_foriter_body_pc` | ||
| /// resolves it through the CALLER's pc tables and answers with a Python pc that | ||
| /// belongs to neither loop, so a callee loop's item is stashed under an | ||
| /// identity no resume coordinate can match, and a caller loop that happens to | ||
| /// resolve to the same pc has its own in-flight entry truncated away and | ||
| /// replaced by the callee's item. | ||
| pub(crate) fn fbw_foriter_body_from_op_pc<Sym: WalkSym>( | ||
| snapshot_sym: *const Sym, | ||
| ctx: &WalkContext<'_, '_, Sym>, | ||
| op_pc: usize, | ||
| ) -> Option<InflightForiterBody> { | ||
| if let Some(consts) = ctx.inline_callee_consts { | ||
| return Some(InflightForiterBody::Jit { | ||
| jitcode_index: consts.jitcode_index, | ||
| op_pc, | ||
| }); | ||
| } | ||
| let snapshot_sym = ctx.fbw_mode.snapshot_sym; | ||
| if snapshot_sym.is_null() { | ||
| return None; | ||
| } | ||
|
|
@@ -171,7 +190,7 @@ pub(crate) fn fbw_foriter_body_from_op_pc<Sym: WalkSym>( | |
| return None; | ||
| } | ||
| Some(InflightForiterBody::Jit { | ||
| outer_jitcode_index: unsafe { (*sym.jitcode()).index as u32 }, | ||
| jitcode_index: unsafe { (*sym.jitcode()).index as i32 }, | ||
| op_pc, | ||
| }) | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the inspected
.github/workflows/pyre-ci.yml,pyre-check-macosreuses the shared step that invokes defaultpyre/check.py, while the separatecpython-testsjob already runspyre/cpython_tests/run.pywith the same dynasm baseline. Making this stage unconditional by default on darwin-arm64 therefore executes the roughly five-minute, 104-module gate twice on every CI run, doubling the expensive macOS work without adding coverage; disable it in one lane or remove the redundant standalone job.Useful? React with 👍 / 👎.