Skip to content

Commit 84eb8fc

Browse files
committed
builder: emitKwNames → C (89/144 → 90/144 = 62.5%)
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)
1 parent 98411cb commit 84eb8fc

3 files changed

Lines changed: 59 additions & 15 deletions

File tree

Python/jit/hir/builder.cpp

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3198,25 +3198,28 @@ extern "C" void hir_builder_insert_run_periodic_activities_c(
31983198
b->insertRunPeriodicActivites(f->cfg, check, succ, *fs);
31993199
}
32003200

3201+
extern "C" void hir_builder_emit_kw_names_c(
3202+
void *tc, void *func, void *builder, PyCodeObject *code, int oparg);
3203+
32013204
void HIRBuilder::emitKwNames(
32023205
TranslationContext& tc,
32033206
const BytecodeInstruction& bc_instr) {
3204-
auto index = bc_instr.oparg();
3205-
auto consts_len = PyTuple_Size(code_->co_consts);
3206-
JIT_CHECK(
3207-
index < consts_len,
3208-
"KW_NAMES index {} is greater than co_consts length {}",
3209-
index,
3210-
consts_len);
3211-
JIT_CHECK(
3212-
kwnames_ == nullptr,
3213-
"Trying to save KW_NAMES({}) but previous kwnames_ value wasn't consumed "
3214-
"by a CALL* opcode yet",
3215-
index);
3207+
hir_builder_emit_kw_names_c(
3208+
static_cast<void*>(&tc),
3209+
static_cast<void*>(current_func_),
3210+
static_cast<void*>(this),
3211+
code_,
3212+
bc_instr.oparg());
3213+
}
32163214

3217-
kwnames_ = temps_.AllocateNonStack();
3218-
tc.emitLoadConst(
3219-
kwnames_, Type::fromObject(PyTuple_GET_ITEM(code_->co_consts, index)));
3215+
extern "C" void *hir_builder_get_kwnames(void *builder) {
3216+
auto *b = static_cast<HIRBuilder*>(builder);
3217+
return static_cast<void*>(b->kwnames_);
3218+
}
3219+
3220+
extern "C" void hir_builder_set_kwnames(void *builder, void *reg) {
3221+
auto *b = static_cast<HIRBuilder*>(builder);
3222+
b->kwnames_ = static_cast<Register*>(reg);
32203223
}
32213224

32223225
extern "C" int hir_builder_emit_binary_op_c(void *tc, void *func, int opcode, int oparg, int specialized_opcode);

Python/jit/hir/builder.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ void *hir_builder_preloader_py_type(void *builder, PyObject *descr);
2424
void hir_builder_insert_run_periodic_activities_c(
2525
void *builder, void *func,
2626
void *check_block, void *succ_block, void *frame_state);
27+
void *hir_builder_get_kwnames(void *builder);
28+
void hir_builder_set_kwnames(void *builder, void *reg);
2729
} // extern "C"
2830
#include <vector>
2931

@@ -108,6 +110,8 @@ class HIRBuilder {
108110
friend void* ::hir_builder_get_block_at_off(void*, int);
109111
friend void ::hir_builder_insert_run_periodic_activities_c(
110112
void*, void*, void*, void*, void*);
113+
friend void* ::hir_builder_get_kwnames(void*);
114+
friend void ::hir_builder_set_kwnames(void*, void*);
111115
public:
112116
const Preloader& preloader() const { return preloader_; }
113117
explicit HIRBuilder(const Preloader& preloader)

Python/jit/hir/builder_emit_c.c

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -960,6 +960,43 @@ void hir_builder_emit_resume_c(
960960
/* tc->block already == succ_block (final state matches C++ tc.block = succ.block) */
961961
}
962962

963+
/* emitKwNames — KW_NAMES opcode handler. Saves the keyword-names tuple
964+
* (from co_consts[oparg]) into the HIRBuilder kwnames_ slot for the next
965+
* CALL/CALL_KW to consume as part of the call's operands. Mirrors C++
966+
* HIRBuilder::emitKwNames @ builder.cpp:3201.
967+
*
968+
* kwnames_ is a NON-STACK temp register on HIRBuilder (one slot, consumed
969+
* by the next CALL opcode). emitKwNames asserts kwnames_ is empty before
970+
* overwriting (matches C++ JIT_CHECK invariant). Bridge access to kwnames_
971+
* via getter/setter (hir_builder_get_kwnames / hir_builder_set_kwnames) so
972+
* future emitCall conversion can also access this state.
973+
*
974+
* AllocateNonStack equivalent: hir_func_alloc_register(func) — same path
975+
* (TempAllocator::AllocateNonStack just calls env_->AllocateRegister at
976+
* builder.cpp:298). emitLoadConst equivalent: hir_c_create_load_const(reg, type)
977+
* + phx_tc_emit. Type::fromObject equivalent: hir_type_from_object(obj). */
978+
extern void *hir_builder_get_kwnames(void *builder);
979+
extern void hir_builder_set_kwnames(void *builder, void *reg);
980+
981+
void hir_builder_emit_kw_names_c(
982+
PhxTranslationContext *tc, void *func, void *builder,
983+
PyCodeObject *code, int oparg) {
984+
Py_ssize_t consts_len = PyTuple_Size(code->co_consts);
985+
JIT_CHECK_C(oparg < consts_len,
986+
"KW_NAMES index %d is greater than co_consts length %zd",
987+
oparg, consts_len);
988+
JIT_CHECK_C(hir_builder_get_kwnames(builder) == NULL,
989+
"Trying to save KW_NAMES(%d) but previous kwnames_ value wasn't "
990+
"consumed by a CALL* opcode yet",
991+
oparg);
992+
993+
void *kwnames_reg = hir_func_alloc_register(func);
994+
hir_builder_set_kwnames(builder, kwnames_reg);
995+
PyObject *names_tuple = PyTuple_GET_ITEM(code->co_consts, oparg);
996+
HirType type = hir_type_from_object(names_tuple);
997+
phx_tc_emit(tc, hir_c_create_load_const(kwnames_reg, type));
998+
}
999+
9631000
/* emitLoadAttr generic — non-specialized LoadAttr2 fallback */
9641001
extern void *hir_c_create_load_attr_reg2(void *dst, void *receiver, int32_t name_idx, void *fs);
9651002

0 commit comments

Comments
 (0)