| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
Patch for Python 3.5 is slightly differ from this PR. |
Sorry, something went wrong.
There was a problem hiding this comment.
Just as I said this is somewhat misleading and it's better to make it clear.
Sorry, something went wrong.
Codecov Report@@ Coverage Diff @@
## master #17 +/- ##
==========================================
+ Coverage 82.37% 82.37% +<.01%
==========================================
Files 1427 1427
Lines 350948 350948
==========================================
+ Hits 289088 289095 +7
+ Misses 61860 61853 -7Continue to review full report at Codecov.
|
Sorry, something went wrong.
Win arm32 master
The `html_url` field takes us to the actual comment on GitHub. `url` field took us to an API JSON response.
Grab builtins dict from module
Also make PyObject an UnsafeCell<ffi::_object> so it can be passed around by & reference
Env initialization used hir_chase_assign (only follows Assign instrs) to compute model registers for support bit assignment. But the pass processing uses phx_rc_model_reg (follows ALL passthroughs except GuardIs). When a phi input goes through a non-Assign passthrough (CheckField, CheckVar, etc.), the two functions return different model registers. The env assigns a bit for the chase_assign model, but the pass looks up using the modelReg model — returning -1 (not found). This -1 was then cast to size_t (becoming SIZE_MAX) and passed to phx_bv_set_bit, causing out-of-bounds memory access → SIGSEGV. Fix: use phx_rc_model_reg in env initialization, matching the pass. Bug python#17: triggered by nbody's phi inputs through non-Assign passthrough instructions (list subscript operations producing CheckVar intermediaries).
Push 47 (88/144 → 89/144 = 61.8%): bundled two-section commit per supervisor 04:48:57Z + theologian 04:48:55Z BUNDLE APPROVED. Section 1 closes a gatekeeper item python#17 violation from push 46; Section 2 ports emitResume to C. ================================================================ SECTION 1 — FOLLOW-UP FIX (item python#17): JIT_CHECK_C in emitTypeAnnotationGuards ================================================================ Push 46 (5f79084) ported emitTypeAnnotationGuards but the C body used a silent-skip workaround for the NULL-arg case: if (arg == NULL) continue; // BAD — silent skip, no diagnostic This violates the no-workarounds policy (feedback_no_workarounds.md): always root-cause, never bail-out/deopt. Supervisor 04:35:30Z + theologian 04:36:18Z + gatekeeper item python#17 (post-push-46 formalization at 04:37:14Z) mandate ALWAYS-ON form (NOT JIT_DCHECK_C, NOT silent-skip): JIT_CHECK_C(arg != NULL, "No register for argument %d", arg_idx); C++ baseline used JIT_CHECK with the same message format (builder.cpp:1751 pre-deletion). Phoenix C parity restored. Match-strings (gatekeeper item python#17 grep targets): - 'JIT_CHECK_C(arg != NULL' ← present - 'No register for argument' ← present Header dependency: added '#include "cinderx/Common/jit_log_c.h"' to builder_emit_c.c for the JIT_CHECK_C macro. No transitive include path existed. ================================================================ SECTION 2 — EMIT METHOD CONVERSION: emitResume → C (88/144 → 89/144) ================================================================ Bridge: hir_builder_emit_resume_c Purpose: RESUME opcode handler — inserts an eval-breaker check + periodic tasks block when oparg < 2. Oparg 2/3 are no-op resumes (coroutine sends/throws don't need periodic-task points). C++ source: builder.cpp:3175-3186 (replaced with 8-line delegating stub) PHASE 0 AUDIT: Type A (headers — all existing at HEAD): - PhxTranslationContext (builder_emit_c.c:22, local typedef) - HirFrameStateLayout (phx_frame_state.h) - Function/CFG/BasicBlock/FrameState (hir.h, used via void* cast in bridge) Type B (symbols — all existing at HEAD): - hir_cfg_alloc_block(void *func) — hir_c_api.h:746 ✓ EXISTS - hir_c_create_snapshot(void *frame_state) — copies FrameState (hir_c_api.cpp:926-934) ✓ EXISTS - phx_tc_emit(tc, instr) — local helper (builder_emit_c.c:39) ✓ EXISTS - HIRBuilder::insertRunPeriodicActivites — private member (builder.h:504-508). NEW friend bridge added: hir_builder_insert_run_periodic_activities_c mirrors hir_builder_get_block_at_off pattern (builder.h:18 fwd-decl + builder.h:105 friend ref). Phase 0.5 IFDEF AUDIT: - insertRunPeriodicActivites contains '#ifdef Py_GIL_DISABLED' block (line 5602-5604 emitAtQuiescentState). This stays in C++ helper — bridge inherits behavior automatically. NO ifdef in new C body. Phase 0 LESSON LEARNED (added during compile-check iteration): Initial attempt missed the file-scope FORWARD DECLARATION prerequisite for friend bridges. Audit must check BOTH: (a) friend-decl site (inside class) (b) file-scope forward-decl site (extern "C" block before class) Will codify in BRIDGE SPEC TEMPLATE meta-rules. PRIOR DECISIONS: - Bundle approved: supervisor 04:48:57Z + theologian 04:48:55Z (47.1 surgical 3-line fix; split would require unsafe stash-dance). - emitResume choice over emitKwNames: theologian's table LOW + LOW vs LOW + LOW-MED — pure velocity contribution per supervisor 04:14:48Z REBALANCE directive (no G-item closure value, that's fine). - Snapshot-on-succ-block trick (avoids temp PhxTranslationContext + frame copy/destroy): theologian 04:45:32Z verified equivalence using hir_c_create_snapshot copy semantics at hir_c_api.cpp:926-934 (snapshot captures frame value at call, subsequent mutations don't affect the snapshot — equivalent to C++ succ.frame semantics). INVARIANTS PRESERVED: 1. Early-return on oparg >= 2 (no Resume action — matches C++ guard). 2. Snapshot emitted into succ_block, NOT check_block (matches C++ succ.emitSnapshot semantic). Trick: capture original tc->block, switch tc->block = succ_block, emit snapshot, then restore via end-state assignment (final tc->block == succ_block). 3. insertRunPeriodicActivites runs full helper chain (eval-breaker check + body w/ RunPeriodicTasks + branches) — preserved via extern "C" delegation through hir_builder_insert_run_periodic_activities_c. 4. Py_GIL_DISABLED behavior preserved (ifdef stays in C++ helper). 5. Final tc->block == succ_block (caller continues on new block — matches C++ tc.block = succ.block). DESIGN EQUIVALENCE PROOF (snapshot-on-succ-block): C++: TranslationContext succ(cfg.AllocateBlock(), tc.frame); succ.emitSnapshot(); insertRunPeriodicActivites(cfg, tc.block, succ.block, tc.frame); tc.block = succ.block; C: void *check_block = tc->block; void *succ_block = hir_cfg_alloc_block(func); tc->block = succ_block; phx_tc_emit(tc, hir_c_create_snapshot(&tc->frame)); hir_builder_insert_run_periodic_activities_c( builder, func, check_block, succ_block, &tc->frame); EQUIVALENT because: (a) C++ succ.frame is a COPY of tc.frame at construction; emitSnapshot doesn't mutate frame; therefore succ.frame == tc.frame at point of insertRunPeriodicActivites call. (b) hir_c_create_snapshot makes a COPY of the FrameState (hir_c_api.cpp:926-934) — snapshot captures the value at the call, not a reference. Subsequent mutations to tc->frame do not affect the snapshot. (c) Snapshot lands in tc->block at time of phx_tc_emit, which we set to succ_block — same destination as C++ succ.emitSnapshot. (d) Final tc->block == succ_block (matches C++ tc.block = succ.block). Falsifier: any function with RESUME opcode (most functions, since RESUME is emitted at function entry by Python 3.12 compiler). JIT-compile, verify HIR shows 2-block sequence around RESUME(0|1): check_block ends with CondBranch on EvalBreaker, succ_block starts with Snapshot. If single-block or wrong successor, port has stripped invariants. ================================================================ DIFF ================================================================ Python/jit/hir/builder.cpp (-11 / +20): - emitResume body (10 lines) → 8-line delegating stub calling extern "C" hir_builder_emit_resume_c. - +13 lines: extern "C" hir_builder_insert_run_periodic_activities_c bridge wrapper (5-line static_cast chain calling private member). Python/jit/hir/builder.h (+5): - +3 lines: file-scope forward declaration of hir_builder_insert_run_periodic_activities_c inside existing extern "C" block (matches hir_builder_get_block_at_off pattern, line 18). - +2 lines: friend declaration inside class HIRBuilder (line 106-107). Python/jit/hir/builder_emit_c.c (+39 / -5): - +1 #include "cinderx/Common/jit_log_c.h" (JIT_CHECK_C macro for §1) - §1 fix: silent-skip replaced with JIT_CHECK_C(arg != NULL, ...) - +35 lines: hir_builder_emit_resume_c implementation + extern decls. Net diff stat: 3 files, +64/-12 (net +52 LOC). ================================================================ VERIFICATION (compile-clean pre-commit) ================================================================ cmake --build phoenix_jit: PASS, 0 errors (only 4 pre-existing warnings). Verified by testkeeper compile-check at 05:02:51Z (after one fix iteration for missing file-scope forward-decl). Build succeeded with friend declaration resolving correctly to global namespace function. Push 47 batch is 1 commit per BUNDLE APPROVED at 04:48:57Z. ABBA_pre baseline 1.07x captured at HEAD 5f79084 (testkeeper 04:59:13Z) for cap-20 ABBA_post comparison. Process discipline applied: - explicit `git add Python/jit/hir/builder.cpp Python/jit/hir/builder.h Python/jit/hir/builder_emit_c.c` (lesson from 6450421c93 over-broad commit; lesson from 04:27:32Z directive-compliance feedback) - `git diff --cached --stat` verification: only 3 files staged - `git status --short` confirmed file separation pre-staging: M Python/jit/hir/builder.cpp (mine, staged) M Python/jit/hir/builder.h (mine, staged) M Python/jit/hir/builder_emit_c.c (mine, staged) M docs/wiring_catches.md (testkeeper W13a, UNSTAGED) M scripts/gate_phoenix.sh (testkeeper W13a, UNSTAGED)
Push 48 Tier 5 emit-method conversion: port emitKwNames from C++ to C per
supervisor 04:14:48Z REBALANCE directive (velocity focus, single emit-method
push). No deletion-gate item closure (post-G2 velocity arc).
================================================================
BRIDGE SPEC TEMPLATE
================================================================
Bridge: hir_builder_emit_kw_names_c
Purpose: KW_NAMES opcode handler — saves the keyword-names tuple from
co_consts[oparg] into the HIRBuilder kwnames_ slot for the next
CALL/CALL_KW to consume as part of the call's operands.
C++ source: builder.cpp:3201-3220 (deleted in this commit)
PHASE 0 AUDIT:
Type A (headers — all existing at HEAD):
- PyCodeObject (Python.h) — co_consts access
- PyTupleObject — PyTuple_Size, PyTuple_GET_ITEM
- PhxTranslationContext (builder_emit_c.c:22, local typedef)
Type B (symbols — all existing at HEAD):
- hir_func_alloc_register(func) ✓ EXISTS — equivalent to
TempAllocator::AllocateNonStack since the latter just calls
env_->AllocateRegister at builder.cpp:298. NO new bridge needed.
- hir_c_create_load_const(reg, type) ✓ EXISTS (builder_emit_c.c:723)
- hir_type_from_object(PyObject*) ✓ EXISTS — equivalent of Type::fromObject
- phx_tc_emit(tc, instr) ✓ EXISTS (local helper, builder_emit_c.c:39)
- JIT_CHECK_C ✓ EXISTS (header included since push 47)
Type B (symbols — NEW BRIDGES, 2):
- hir_builder_get_kwnames(builder) → void* (Register*)
- hir_builder_set_kwnames(builder, void *reg)
Reason for getter+setter pair (vs. single 'do everything' bridge):
emitCall (next conversion target after kwnames cluster) reads kwnames_
and clears it post-consume (builder.cpp:3088-3093). Independent
getter+setter enables that future conversion without re-architecting.
Phase 0.5 IFDEF AUDIT: N/A (no #ifdef-guarded code in emitKwNames)
Phase 0 LESSON FROM PUSH 47 APPLIED:
Both new bridges have:
(a) friend declarations in HIRBuilder class (builder.h:108-109)
(b) file-scope forward declarations in extern "C" block (builder.h:28-29)
Both sites verified BEFORE first compile-check this time.
No friend-decl resolution error (vs. push 47 first attempt).
PRIOR DECISIONS:
- emitKwNames choice over CLUSTER (post-G2 velocity arc): supervisor
05:13:54Z + theologian 04:14:15Z pre-analysis (LOW + LOW-MED). No
theologian CLUSTER analysis posted before push 48 announcement, so
default per supervisor.
- LOW-MED risk caveat is about kwnames touching CALL_KW argument-passing
semantics. emitKwNames is the PRODUCER side (saves the names tuple);
the CONSUMER (emitCall) is unchanged in this push. So push 48 risk
surface is producer-only path, NOT call-graph integration.
INVARIANTS PRESERVED:
1. Index bounds check: oparg < co_consts length, otherwise abort with
diagnostic. Matches C++ JIT_CHECK at builder.cpp:3206-3210.
Implemented as JIT_CHECK_C (ALWAYS-ON, item python#17 form).
2. Single-slot invariant: kwnames_ MUST be NULL when emitKwNames runs;
otherwise prior KW_NAMES wasn't consumed by a CALL* opcode. Matches
C++ JIT_CHECK at builder.cpp:3211-3215. Implemented as JIT_CHECK_C.
3. Non-stack allocation: kwnames_reg is allocated as a non-stack temp
(hir_func_alloc_register, NOT phx_ptr_arr_push to stack). Matches
C++ temps_.AllocateNonStack semantic.
4. LoadConst with object-type: tuple type captured via
hir_type_from_object (Type::fromObject equivalent) — gives the
consumer (emitCall) access to the full PyObject spec for
resolveKwargs static-resolution path.
5. NO stack push: kwnames_ is NOT a stack value. The reg lives on the
HIRBuilder for the next CALL to read. emitLoadConst on TC vs. the
stack-pushing path (hir_builder_emit_load_const_c) is a critical
difference — hir_c_create_load_const + phx_tc_emit alone (no
phx_ptr_arr_push to stack).
Falsifier: Python function with keyword-arg call (e.g. `f(x=1, y=2)`).
JIT-compile, verify HIR shows: KW_NAMES instruction emits LoadConst<TTuple>
to a non-stack reg, followed by CALL with that reg as last operand. If
the kwnames reg appears on the stack OR if CALL receives wrong tuple, port
has stripped invariants.
================================================================
DIFF
================================================================
Python/jit/hir/builder.cpp (-15 / +33):
- emitKwNames body (~16 lines) → 8-line delegating stub calling
extern "C" hir_builder_emit_kw_names_c.
- +12 lines: extern "C" hir_builder_get_kwnames + hir_builder_set_kwnames
bridge wrappers (3 lines each, static_cast + field access).
Python/jit/hir/builder.h (+4):
- +2 lines: file-scope forward decls of hir_builder_get_kwnames +
hir_builder_set_kwnames inside existing extern "C" block (matches
pattern from push 47).
- +2 lines: friend decls inside class HIRBuilder.
Python/jit/hir/builder_emit_c.c (+37):
- +2 extern decls (kwnames getter/setter)
- hir_builder_emit_kw_names_c implementation (~22 lines)
- Comment block describing semantics + bridge rationale.
Net diff stat: 3 files, +59/-15 (net +44 LOC).
================================================================
VERIFICATION (compile-clean pre-commit)
================================================================
cmake --build phoenix_jit: PASS, 0 errors (only 4 pre-existing warnings).
Verified by testkeeper compile-check at 05:19:22Z.
Friend declaration + file-scope forward decl both resolved correctly
(Phase 0 audit refinement from push 47 applied successfully).
Push 48 batch is 1 commit per supervisor 04:14:48Z REBALANCE directive
(single emit-method push, velocity focus).
Process discipline applied:
- explicit `git add Python/jit/hir/builder.cpp Python/jit/hir/builder.h
Python/jit/hir/builder_emit_c.c` (lesson from 6450421c93 over-broad
commit; lesson from 04:27:32Z directive-compliance feedback)
- `git diff --cached --stat` verification: only 3 files staged
- `git status --short` confirmed file separation pre-staging:
M Python/jit/hir/builder.cpp (mine, staged)
M Python/jit/hir/builder.h (mine, staged)
M Python/jit/hir/builder_emit_c.c (mine, staged)
M docs/wiring_catches.md (testkeeper W13a, UNSTAGED)
M scripts/gate_phoenix.sh (testkeeper W13a, UNSTAGED)
| Back | FazBrowse Home | New Git URL |
No description provided.