Skip to content

Commit a93cbe7

Browse files
ddorianddorian
committed
Guard keys version only on upstream Python
Meta Python invalidates the type version when shared keys change, so _GUARD_TYPE_VERSION covers it there; check dk_version under #ifndef META_PYTHON. Move the regression tests into test_jit_specialization.py.
1 parent b593d91 commit a93cbe7

6 files changed

Lines changed: 136 additions & 116 deletions

File tree

‎cinderx/Interpreter/3.14/Includes/generated_cases.c.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10062,6 +10062,7 @@
1006210062
// _GUARD_KEYS_VERSION
1006310063
{
1006410064
uint32_t keys_version = read_u32(&this_instr[4].cache);
10065+
#ifndef META_PYTHON
1006510066
PyTypeObject *owner_cls = Py_TYPE(PyStackRef_AsPyObjectBorrow(owner));
1006610067
PyHeapTypeObject *owner_heap_type = (PyHeapTypeObject *)owner_cls;
1006710068
PyDictKeysObject *keys = owner_heap_type->ht_cached_keys;
@@ -10070,6 +10071,9 @@
1007010071
assert(_PyOpcode_Deopt[opcode] == (LOAD_ATTR));
1007110072
JUMP_TO_PREDICTED(LOAD_ATTR);
1007210073
}
10074+
#else
10075+
(void)keys_version;
10076+
#endif
1007310077
}
1007410078
// _LOAD_ATTR_METHOD_WITH_VALUES
1007510079
{
@@ -10256,6 +10260,7 @@
1025610260
// _GUARD_KEYS_VERSION
1025710261
{
1025810262
uint32_t keys_version = read_u32(&this_instr[4].cache);
10263+
#ifndef META_PYTHON
1025910264
PyTypeObject *owner_cls = Py_TYPE(PyStackRef_AsPyObjectBorrow(owner));
1026010265
PyHeapTypeObject *owner_heap_type = (PyHeapTypeObject *)owner_cls;
1026110266
PyDictKeysObject *keys = owner_heap_type->ht_cached_keys;
@@ -10264,6 +10269,9 @@
1026410269
assert(_PyOpcode_Deopt[opcode] == (LOAD_ATTR));
1026510270
JUMP_TO_PREDICTED(LOAD_ATTR);
1026610271
}
10272+
#else
10273+
(void)keys_version;
10274+
#endif
1026710275
}
1026810276
// _LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES
1026910277
{

‎cinderx/Interpreter/3.14/cinder-bytecodes.c‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,33 @@ dummy_func(
583583
#endif
584584
}
585585

586+
// Meta Python bumps the type version when shared keys change, so
587+
// _GUARD_TYPE_VERSION covers it; upstream Python still needs this check.
588+
op(_GUARD_KEYS_VERSION, (keys_version/2, owner -- owner)) {
589+
#ifndef META_PYTHON
590+
PyTypeObject *owner_cls = Py_TYPE(PyStackRef_AsPyObjectBorrow(owner));
591+
PyHeapTypeObject *owner_heap_type = (PyHeapTypeObject *)owner_cls;
592+
PyDictKeysObject *keys = owner_heap_type->ht_cached_keys;
593+
DEOPT_IF(FT_ATOMIC_LOAD_UINT32_RELAXED(keys->dk_version) != keys_version);
594+
#else
595+
(void)keys_version;
596+
#endif
597+
}
598+
599+
macro(LOAD_ATTR_METHOD_WITH_VALUES) =
600+
unused/1 +
601+
_GUARD_TYPE_VERSION +
602+
_GUARD_DORV_VALUES_INST_ATTR_FROM_DICT +
603+
_GUARD_KEYS_VERSION +
604+
_LOAD_ATTR_METHOD_WITH_VALUES;
605+
606+
macro(LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES) =
607+
unused/1 +
608+
_GUARD_TYPE_VERSION +
609+
_GUARD_DORV_VALUES_INST_ATTR_FROM_DICT +
610+
_GUARD_KEYS_VERSION +
611+
_LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES;
612+
586613
override inst(EXTENDED_OPCODE, (args[oparg>>2] -- top[oparg&0x03])) {
587614
// Decode any extended oparg
588615
int extop = (int)next_instr->op.code;

‎cinderx/Interpreter/3.15/Includes/generated_cases.c.h‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10243,14 +10243,18 @@
1024310243
// _GUARD_KEYS_VERSION
1024410244
{
1024510245
uint32_t keys_version = read_u32(&this_instr[4].cache);
10246-
PyTypeObject *owner_cls = Py_TYPE(PyStackRef_AsPyObjectBorrow(owner));
10247-
PyHeapTypeObject *owner_heap_type = (PyHeapTypeObject *)owner_cls;
10248-
PyDictKeysObject *keys = owner_heap_type->ht_cached_keys;
10246+
#ifndef META_PYTHON
10247+
PyTypeObject* owner_cls = Py_TYPE(PyStackRef_AsPyObjectBorrow(owner));
10248+
PyHeapTypeObject* owner_heap_type = (PyHeapTypeObject*)owner_cls;
10249+
PyDictKeysObject* keys = owner_heap_type->ht_cached_keys;
1024910250
if (FT_ATOMIC_LOAD_UINT32_RELAXED(keys->dk_version) != keys_version) {
1025010251
UPDATE_MISS_STATS(LOAD_ATTR);
1025110252
assert(_PyOpcode_Deopt[opcode] == (LOAD_ATTR));
1025210253
JUMP_TO_PREDICTED(LOAD_ATTR);
1025310254
}
10255+
#else
10256+
(void)keys_version;
10257+
#endif
1025410258
}
1025510259
// _LOAD_ATTR_METHOD_WITH_VALUES
1025610260
{
@@ -10437,14 +10441,18 @@
1043710441
// _GUARD_KEYS_VERSION
1043810442
{
1043910443
uint32_t keys_version = read_u32(&this_instr[4].cache);
10440-
PyTypeObject *owner_cls = Py_TYPE(PyStackRef_AsPyObjectBorrow(owner));
10441-
PyHeapTypeObject *owner_heap_type = (PyHeapTypeObject *)owner_cls;
10442-
PyDictKeysObject *keys = owner_heap_type->ht_cached_keys;
10444+
#ifndef META_PYTHON
10445+
PyTypeObject* owner_cls = Py_TYPE(PyStackRef_AsPyObjectBorrow(owner));
10446+
PyHeapTypeObject* owner_heap_type = (PyHeapTypeObject*)owner_cls;
10447+
PyDictKeysObject* keys = owner_heap_type->ht_cached_keys;
1044310448
if (FT_ATOMIC_LOAD_UINT32_RELAXED(keys->dk_version) != keys_version) {
1044410449
UPDATE_MISS_STATS(LOAD_ATTR);
1044510450
assert(_PyOpcode_Deopt[opcode] == (LOAD_ATTR));
1044610451
JUMP_TO_PREDICTED(LOAD_ATTR);
1044710452
}
10453+
#else
10454+
(void)keys_version;
10455+
#endif
1044810456
}
1044910457
// _LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES
1045010458
{

‎cinderx/Interpreter/3.15/cinder-bytecodes.c‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,27 @@ static PyObject* dummy_func(
191191
ERROR_IF(err < 0);
192192
}
193193

194+
// Meta Python bumps the type version when shared keys change, so
195+
// _GUARD_TYPE_VERSION covers it; upstream Python still needs this check.
196+
op(_GUARD_KEYS_VERSION, (keys_version / 2, owner-- owner)) {
197+
#ifndef META_PYTHON
198+
PyTypeObject* owner_cls = Py_TYPE(PyStackRef_AsPyObjectBorrow(owner));
199+
PyHeapTypeObject* owner_heap_type = (PyHeapTypeObject*)owner_cls;
200+
PyDictKeysObject* keys = owner_heap_type->ht_cached_keys;
201+
DEOPT_IF(FT_ATOMIC_LOAD_UINT32_RELAXED(keys->dk_version) != keys_version);
202+
#else
203+
(void)keys_version;
204+
#endif
205+
}
206+
207+
macro(LOAD_ATTR_METHOD_WITH_VALUES) = unused / 1 + _GUARD_TYPE_VERSION +
208+
_GUARD_DORV_VALUES_INST_ATTR_FROM_DICT + _GUARD_KEYS_VERSION +
209+
_LOAD_ATTR_METHOD_WITH_VALUES;
210+
211+
macro(LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES) = unused / 1 +
212+
_GUARD_TYPE_VERSION + _GUARD_DORV_VALUES_INST_ATTR_FROM_DICT +
213+
_GUARD_KEYS_VERSION + _LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES;
214+
194215
override inst(EXTENDED_OPCODE, (args[oparg >> 2]-- top[oparg & 0x03])) {
195216
// Decode any extended oparg
196217
int extop = (int)next_instr->op.code;

‎cinderx/PythonLib/test_cinderx/test_jit_specialization.py‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@
22

33
# pyre-strict
44

5+
import contextlib
56
import dis
67
import sys
78
import unittest
89
from types import ModuleType
9-
from typing import Callable, TypeVar
10+
from typing import Callable, Iterator, TypeVar
1011

1112
import cinderx
1213
import cinderx.jit
@@ -51,6 +52,31 @@ def specialize(
5152
cinderx.jit.force_compile(func)
5253

5354

55+
# The specialized opcodes under test only run in CinderX's eval loop, which
56+
# needs the frame evaluator installed.
57+
@contextlib.contextmanager
58+
def frame_evaluator() -> Iterator[None]:
59+
if cinderx.is_frame_evaluator_installed():
60+
yield
61+
return
62+
63+
cinderx.install_frame_evaluator()
64+
try:
65+
yield
66+
finally:
67+
cinderx.remove_frame_evaluator()
68+
69+
70+
# Like specialize(), but stays interpreted so the interpreter's guards run.
71+
def specialize_interpreted(
72+
func: Callable[..., TCallableRet], callable: Callable[[], TCallableRet]
73+
) -> None:
74+
cinderx.jit.jit_suppress(func)
75+
76+
for _ in range(5):
77+
callable()
78+
79+
5480
@passIf(not cinderx.jit.is_enabled(), "Tests functionality on the JIT")
5581
class SpecializationTests(unittest.TestCase):
5682
def setUp(self) -> None:
@@ -298,6 +324,45 @@ def f() -> str:
298324
self.assertIn("LOAD_ATTR_MODULE", opnames(f))
299325
self.assertEqual(f(), sys.argv[0])
300326

327+
@passUnless(sys.version_info >= (3, 14), "3.12 only builds against Meta Python")
328+
def test_load_attr_nondescriptor_with_values(self) -> None:
329+
class C:
330+
attr: object = "class-attr"
331+
332+
def f(o: C) -> object:
333+
return o.attr
334+
335+
with frame_evaluator():
336+
specialize_interpreted(f, lambda: f(C()))
337+
338+
self.assertNotIn("LOAD_ATTR", opnames(f))
339+
self.assertIn("LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES", opnames(f))
340+
341+
# Setting the attribute on an instance bumps the shared keys, so the
342+
# load must deopt instead of returning the cached class attribute.
343+
o = C()
344+
o.attr = "instance-attr"
345+
self.assertEqual(f(o), "instance-attr")
346+
347+
@passUnless(sys.version_info >= (3, 14), "3.12 only builds against Meta Python")
348+
def test_load_attr_method_with_values(self) -> None:
349+
class C:
350+
def m(self) -> str:
351+
return "class-method"
352+
353+
def f(o: C) -> str:
354+
return o.m()
355+
356+
with frame_evaluator():
357+
specialize_interpreted(f, lambda: f(C()))
358+
359+
self.assertNotIn("LOAD_ATTR", opnames(f))
360+
self.assertIn("LOAD_ATTR_METHOD_WITH_VALUES", opnames(f))
361+
362+
o = C()
363+
setattr(o, "m", lambda: "instance-attr")
364+
self.assertEqual(f(o), "instance-attr")
365+
301366
def test_store_subscr_dict(self) -> None:
302367
def f(a: dict[str, str], b: str, c: str) -> None:
303368
a[b] = c

‎cinderx/PythonLib/test_cinderx/test_load_attr_keys_version.py‎

Lines changed: 0 additions & 109 deletions
This file was deleted.

0 commit comments

Comments
 (0)