Skip to content

Commit 1553c14

Browse files
SonicFieldSonicField
committed
W26 fix: dispatch via PhxCallKind enum (W21 golden regression closure)
W26 Step B (95c9f9b) introduced a W21 golden HIR regression on both arches per testkeeper L2484: attr_probe BinaryOp<Add> was not being specialized into LongBinaryOp<Add> + GuardType in the post-W26 build. ROOT CAUSE: my W26 added cinder_opcode.h BEFORE opcode.h in builder_emit_c.c to make INVOKE_FUNCTION/_NATIVE/_METHOD opcode constants available for the case labels in the new emit_any_call C body switch. But cinder_opcode.h's cinder_opcode_ids.h sets Py_OPCODE_H header guard which then SHADOWED Include/opcode.h, leaving BINARY_OP_ADD_INT undefined. The BINARY_OP specialization path at builder_emit_c.c:1135 uses `#ifdef BINARY_OP_ADD_INT` to gate its GuardType + LongBinaryOp emission — silently compiled out when BINARY_OP_ADD_INT was undefined. attr_probe's a + b + c chain (3 LOAD_ATTR + 2 BINARY_OP_ADD_INT specializations) lost its 4 GuardType emits + 2 LongBinaryOp specializations, leaving only 2 generic BinaryOp<Add> ops. W21 golden test (push 62) was added EXPLICITLY to catch HIR drift in emit-method conversions. It fired the trip wire on the first attempt — discipline working as intended. FIX (PhxCallKind enum dispatch): - C body switches on `int call_kind` (PhxCallKind enum value) instead of on opcode constants. Constants defined as #defines in builder_emit_c.c + matching `enum PhxCallKind` in builder.h. - C++ stub maps opcode → PhxCallKind via a small switch (CALL_FUNCTION → VECTOR_CALL, CALL_KW → CALL_METHOD+is_kw_arg, etc.) before delegation. is_kw_arg is a separate sub-flag for opcodes that need the kwnames adjustment (CALL_FUNCTION_KW + CALL_KW). - REVERTED the cinder_opcode.h include that broke opcode.h's #defines. - Kept opcode_stubs.h include for YIELD_FROM (used in await-tail JIT_CHECK_C; not in 3.12 Include/opcode.h, only in stubs). Empirical verification: - cmake --build target jit/phoenix_jit clean (BUILD_EXIT=0) - ./python -m test test_phoenix_jit_loadattr_golden → PASS (was FAIL) - ./python -m test test_phoenix_jit_inline_except_closure → 6/6 PASS W26 Step B + this fix together: full conversion of emitAnyCall to C + 149b7e2 PartialConversion REABSORB, with W21 golden parity.
1 parent 95c9f9b commit 1553c14

3 files changed

Lines changed: 84 additions & 28 deletions

File tree

‎Python/jit/hir/builder.cpp‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2913,8 +2913,8 @@ extern "C" int hir_builder_bc_it_oparg_c(void *bc_it) {
29132913
extern "C" void hir_builder_emit_any_call_c(
29142914
void *tc, void *cfg, void *func, void *builder,
29152915
void *bc_instrs, void *bc_it,
2916-
int opcode, int oparg, int base_offset,
2917-
int is_awaited,
2916+
int call_kind, int oparg, int base_offset,
2917+
int is_awaited, int is_kw_arg,
29182918
void *code, int code_flags,
29192919
PyObject *const_arg);
29202920

@@ -2928,6 +2928,34 @@ void HIRBuilder::emitAnyCall(
29282928
int oparg = bc_instr.oparg();
29292929
int base_offset = bc_instr.baseOffset().value();
29302930

2931+
// Map opcode → PhxCallKind (C body switches on enum to avoid pulling
2932+
// opcode constants into builder_emit_c.c, where cinder_opcode.h's
2933+
// Py_OPCODE_H header guard would shadow Include/opcode.h's
2934+
// BINARY_OP_ADD_INT define + break #ifdef in BINARY_OP specialization).
2935+
int call_kind;
2936+
int is_kw_arg = 0;
2937+
switch (opcode) {
2938+
case CALL_FUNCTION:
2939+
call_kind = PHX_CALL_KIND_VECTOR_CALL; break;
2940+
case CALL_FUNCTION_KW:
2941+
call_kind = PHX_CALL_KIND_VECTOR_CALL; is_kw_arg = 1; break;
2942+
case CALL_FUNCTION_EX:
2943+
call_kind = PHX_CALL_KIND_CALL_EX; break;
2944+
case CALL:
2945+
case CALL_METHOD:
2946+
call_kind = PHX_CALL_KIND_CALL_METHOD; break;
2947+
case CALL_KW:
2948+
call_kind = PHX_CALL_KIND_CALL_METHOD; is_kw_arg = 1; break;
2949+
case INVOKE_FUNCTION:
2950+
call_kind = PHX_CALL_KIND_INVOKE_FUNCTION; break;
2951+
case INVOKE_NATIVE:
2952+
call_kind = PHX_CALL_KIND_INVOKE_NATIVE; break;
2953+
case INVOKE_METHOD:
2954+
call_kind = PHX_CALL_KIND_INVOKE_METHOD; break;
2955+
default:
2956+
JIT_ABORT("Unhandled call opcode {} ({})", opcode, opcodeName(opcode));
2957+
}
2958+
29312959
int is_awaited;
29322960
if constexpr (PY_VERSION_HEX >= 0x030C0000) {
29332961
is_awaited = 0;
@@ -2953,7 +2981,7 @@ void HIRBuilder::emitAnyCall(
29532981
&tc, &cfg, current_func_, this,
29542982
const_cast<void*>(static_cast<const void*>(&bc_instrs)),
29552983
&bc_it,
2956-
opcode, oparg, base_offset, is_awaited,
2984+
call_kind, oparg, base_offset, is_awaited, is_kw_arg,
29572985
code_, static_cast<int>(code_->co_flags),
29582986
const_arg);
29592987
}

‎Python/jit/hir/builder.h‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,21 @@ void *hir_builder_static_method_stack_pop_c(void *builder);
4444
/* W26 (theologian L2462+L2466): bridges for emitAnyCall full conversion +
4545
* 149b7e2d40 PartialConversion reabsorb. 4 NEW bridges: combined exception-
4646
* handler emit (folds findExceptionHandler+getSimpleExceptInfo+emit per (B)
47-
* decision), checkAsyncWithError, bc_it advance+opcode, bc_it oparg. */
47+
* decision), checkAsyncWithError, bc_it advance+opcode, bc_it oparg.
48+
*
49+
* PhxCallKind: opcode-to-kind mapping done in C++ stub, so the C body can
50+
* dispatch on a small enum without needing opcode constants in scope (which
51+
* would conflict with cinder_opcode.h's Py_OPCODE_H header guard, breaking
52+
* #ifdef BINARY_OP_ADD_INT in the BINARY_OP specialization path — root
53+
* cause of W26 first-attempt W21 golden regression at 95c9f9b891). */
54+
enum PhxCallKind {
55+
PHX_CALL_KIND_VECTOR_CALL = 0, /* CALL_FUNCTION / CALL_FUNCTION_KW */
56+
PHX_CALL_KIND_CALL_EX, /* CALL_FUNCTION_EX */
57+
PHX_CALL_KIND_CALL_METHOD, /* CALL / CALL_KW / CALL_METHOD */
58+
PHX_CALL_KIND_INVOKE_FUNCTION, /* INVOKE_FUNCTION (Cinder static) */
59+
PHX_CALL_KIND_INVOKE_NATIVE, /* INVOKE_NATIVE (Cinder static) */
60+
PHX_CALL_KIND_INVOKE_METHOD /* INVOKE_METHOD (Cinder static) */
61+
};
4862
void hir_builder_emit_call_method_exception_handler_inline_c(
4963
void *builder, void *tc, void *cfg, int base_offset,
5064
void *call_instr, void *result_reg);

‎Python/jit/hir/builder_emit_c.c‎

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,20 @@
1717
#include "Python.h"
1818
#include "internal/pycore_moduleobject.h" /* PyModuleObject (LOAD_ATTR_MODULE) */
1919
#include "internal/pycore_dict.h" /* PyDictKeysObject (LOAD_ATTR_MODULE) */
20-
/* cinder_opcode.h MUST be included before opcode.h — it uses the same
21-
* Py_OPCODE_H header guard but defines the cinder-extended opcode enum
22-
* (INVOKE_FUNCTION/_NATIVE/_METHOD). Including opcode.h first would shadow it. */
23-
#include "cinderx/Interpreter/cinder_opcode.h"
2420
#include "opcode.h"
25-
#include "cinderx/Common/opcode_stubs.h" /* CALL_FUNCTION/CALL_FUNCTION_KW/CALL_KW/CALL_METHOD/YIELD_FROM stubs for 3.12 */
21+
#include "cinderx/Common/opcode_stubs.h" /* YIELD_FROM stub for 3.12 (not in Include/opcode.h) */
22+
23+
/* PhxCallKind values — must match enum PhxCallKind in builder.h. C body
24+
* dispatches on these instead of opcode constants to avoid pulling in
25+
* cinder_opcode.h which would shadow Include/opcode.h's BINARY_OP_ADD_INT
26+
* define + break #ifdef in BINARY_OP specialization (W26 first-attempt
27+
* regression root cause). C++ stub maps opcode → kind. */
28+
#define PHX_CALL_KIND_VECTOR_CALL 0
29+
#define PHX_CALL_KIND_CALL_EX 1
30+
#define PHX_CALL_KIND_CALL_METHOD 2
31+
#define PHX_CALL_KIND_INVOKE_FUNCTION 3
32+
#define PHX_CALL_KIND_INVOKE_NATIVE 4
33+
#define PHX_CALL_KIND_INVOKE_METHOD 5
2634

2735
/* ---- PhxTranslationContext ---- */
2836

@@ -3489,27 +3497,33 @@ void hir_builder_emit_any_call_c(
34893497
void *builder,
34903498
void *bc_instrs,
34913499
void *bc_it,
3492-
int opcode,
3500+
int call_kind, /* PhxCallKind enum (mapped from opcode by C++ stub) */
34933501
int oparg,
34943502
int base_offset,
34953503
int is_awaited,
3504+
int is_kw_arg, /* set for CALL_FUNCTION_KW + CALL_KW (kwnames flag) */
34963505
void *code,
34973506
int code_flags,
34983507
PyObject *const_arg) {
3499-
/* CallFlags: None=0, KwArgs=1<<0=1, Awaited=1<<1=2, Static=1<<2=4. */
3508+
/* CallFlags: None=0, KwArgs=1<<0=1, Awaited=1<<1=2, Static=1<<2=4.
3509+
*
3510+
* call_kind dispatch (set by C++ stub) replaces direct opcode switching
3511+
* here so this C body does not need to import opcode constants. The
3512+
* opcode switch in the C++ stub maps each call-class opcode to one of
3513+
* the PHX_CALL_KIND_* values (defined in builder.h). */
35003514
uint32_t flags = is_awaited ? 2u : 0u;
35013515
int call_used_is_awaited = 1;
35023516

3503-
switch (opcode) {
3504-
case CALL_FUNCTION:
3505-
case CALL_FUNCTION_KW: {
3506-
/* Operands include the function arguments plus the function itself. */
3517+
switch (call_kind) {
3518+
case PHX_CALL_KIND_VECTOR_CALL: {
3519+
/* CALL_FUNCTION / CALL_FUNCTION_KW: variadic vector call.
3520+
* Operands include the function arguments plus the function
3521+
* itself. is_kw_arg adds 1 for the kwnames tuple. */
35073522
size_t num_operands = (size_t)(oparg + 1);
3508-
if (opcode == CALL_FUNCTION_KW) {
3523+
if (is_kw_arg) {
35093524
num_operands++;
35103525
flags |= 1u; /* CallFlags::KwArgs */
35113526
}
3512-
/* emitVariadic<VectorCall> equivalent — use existing C primitives. */
35133527
void *out = hir_builder_temps_alloc_stack(builder);
35143528
void *call = hir_c_create_vectorcall_reg(num_operands, out, flags);
35153529
for (size_t i = num_operands; i > 0; i--) {
@@ -3521,19 +3535,19 @@ void hir_builder_emit_any_call_c(
35213535
phx_ptr_arr_push(&tc->frame.stack, out);
35223536
break;
35233537
}
3524-
case CALL_FUNCTION_EX: {
3538+
case PHX_CALL_KIND_CALL_EX: {
35253539
hir_builder_emit_call_ex_c(tc, func, oparg, flags);
35263540
break;
35273541
}
3528-
case CALL:
3529-
case CALL_KW:
3530-
case CALL_METHOD: {
3542+
case PHX_CALL_KIND_CALL_METHOD: {
3543+
/* CALL / CALL_KW / CALL_METHOD: dynamic call-method dispatch
3544+
* with kwnames + exception-handler inline. is_kw_arg tracks
3545+
* whether opcode is CALL_KW (extra stack input + flag). */
35313546
size_t num_operands = (size_t)(oparg + 2);
35323547
size_t num_stack_inputs = num_operands;
3533-
int is_call_kw = (opcode == CALL_KW) ? 1 : 0;
35343548
void *kwnames_reg = hir_builder_get_kwnames(builder);
3535-
if (kwnames_reg != NULL || is_call_kw) {
3536-
if (is_call_kw) {
3549+
if (kwnames_reg != NULL || is_kw_arg) {
3550+
if (is_kw_arg) {
35373551
num_stack_inputs++;
35383552
}
35393553
num_operands++;
@@ -3566,30 +3580,30 @@ void hir_builder_emit_any_call_c(
35663580
builder, tc, cfg, base_offset, call, out);
35673581
break;
35683582
}
3569-
case INVOKE_FUNCTION: {
3583+
case PHX_CALL_KIND_INVOKE_FUNCTION: {
35703584
PyObject *descr = PyTuple_GET_ITEM(const_arg, 0);
35713585
long nargs = PyLong_AsLong(PyTuple_GET_ITEM(const_arg, 1));
35723586
call_used_is_awaited = hir_builder_emit_invoke_function_c(
35733587
tc, func, builder, descr, nargs, flags) ? 1 : 0;
35743588
break;
35753589
}
3576-
case INVOKE_NATIVE: {
3590+
case PHX_CALL_KIND_INVOKE_NATIVE: {
35773591
PyObject *native_target_descr = PyTuple_GET_ITEM(const_arg, 0);
35783592
PyObject *signature = PyTuple_GET_ITEM(const_arg, 1);
35793593
hir_builder_emit_invoke_native_c(tc, builder,
35803594
native_target_descr, signature);
35813595
call_used_is_awaited = 0; /* emitInvokeNative always returns false */
35823596
break;
35833597
}
3584-
case INVOKE_METHOD: {
3598+
case PHX_CALL_KIND_INVOKE_METHOD: {
35853599
PyObject *descr = PyTuple_GET_ITEM(const_arg, 0);
35863600
long nargs = PyLong_AsLong(PyTuple_GET_ITEM(const_arg, 1)) + 2;
35873601
call_used_is_awaited = hir_builder_emit_invoke_method_c(
35883602
tc, func, builder, descr, nargs, is_awaited) ? 1 : 0;
35893603
break;
35903604
}
35913605
default:
3592-
JIT_CHECK_C(0, "Unhandled call opcode %d", opcode);
3606+
JIT_CHECK_C(0, "Unhandled call kind %d", call_kind);
35933607
}
35943608

35953609
if (is_awaited && call_used_is_awaited) {

0 commit comments

Comments
 (0)