Get globals to const pass working

markshannon · Feb 2, 2024 · Jan 18, 2024 · Jan 18, 2024 · Jan 18, 2024 · Jan 18, 2024
commit 35a92cccd3aaabf0e86734bbcdc6e0c3ff7eb8c3
diff --git a/Include/internal/pycore_optimizer.h b/Include/internal/pycore_optimizer.h
@@ -9,7 +9,8 @@ extern "C" {
 #endif
 
 int _Py_uop_analyze_and_optimize(_PyInterpreterFrame *frame,
-    _PyUOpInstruction *trace, int trace_len, int curr_stackentries);
+    _PyUOpInstruction *trace, int trace_len, int curr_stackentries,
+    _PyBloomFilter *dependencies);
 
 extern PyTypeObject _PyCounterExecutor_Type;
 extern PyTypeObject _PyCounterOptimizer_Type;

diff --git a/Lib/test/test_capi/test_watchers.py b/Lib/test/test_capi/test_watchers.py
@@ -52,14 +52,6 @@ def test_set_existing_item(self):
             d["foo"] = "baz"
             self.assert_events(["mod:foo:baz"])
 
-    def test_clone(self):
-        d = {}
-        d2 = {"foo": "bar"}
-        with self.watcher() as wid:
-            self.watch(wid, d)
-            d.update(d2)
-            self.assert_events(["clone"])
-
     def test_no_event_if_not_watched(self):
         d = {}
         with self.watcher() as wid:
@@ -80,6 +72,14 @@ def test_pop(self):
             d.pop("foo")
             self.assert_events(["del:foo"])
 
+    def test_clone(self):
+        d = {}
+        d2 = {"foo": "bar"}
+        with self.watcher() as wid:
+            self.watch(wid, d)
+            d.update(d2)
+            self.assert_events(["clone"])
+
     def test_clear(self):
         d = {"foo": "bar"}
         with self.watcher() as wid:
@@ -151,8 +151,8 @@ def test_watch_out_of_range_watcher_id(self):
 
     def test_watch_unassigned_watcher_id(self):
         d = {}
-        with self.assertRaisesRegex(ValueError, r"No dict watcher set for ID 1"):
-            self.watch(1, d)
+        with self.assertRaisesRegex(ValueError, r"No dict watcher set for ID 3"):
+            self.watch(3, d)
 
     def test_unwatch_non_dict(self):
         with self.watcher() as wid:
@@ -168,8 +168,8 @@ def test_unwatch_out_of_range_watcher_id(self):
 
     def test_unwatch_unassigned_watcher_id(self):
         d = {}
-        with self.assertRaisesRegex(ValueError, r"No dict watcher set for ID 1"):
-            self.unwatch(1, d)
+        with self.assertRaisesRegex(ValueError, r"No dict watcher set for ID 3"):
+            self.unwatch(3, d)
 
     def test_clear_out_of_range_watcher_id(self):
         with self.assertRaisesRegex(ValueError, r"Invalid dict watcher ID -1"):
@@ -178,8 +178,8 @@ def test_clear_out_of_range_watcher_id(self):
             self.clear_watcher(8)  # DICT_MAX_WATCHERS = 8
 
     def test_clear_unassigned_watcher_id(self):
-        with self.assertRaisesRegex(ValueError, r"No dict watcher set for ID 1"):
-            self.clear_watcher(1)
+        with self.assertRaisesRegex(ValueError, r"No dict watcher set for ID 3"):
+            self.clear_watcher(3)
 
 
 class TestTypeWatchers(unittest.TestCase):

diff --git a/Modules/_testcapi/watchers.c b/Modules/_testcapi/watchers.c
@@ -15,8 +15,8 @@ module _testcapi
 /*[clinic end generated code: output=da39a3ee5e6b4b0d input=6361033e795369fc]*/
 
 // Test dict watching
-static PyObject *g_dict_watch_events;
-static int g_dict_watchers_installed;
+static PyObject *g_dict_watch_events = NULL;
+static int g_dict_watchers_installed = 0;
 
 static int
 dict_watch_callback(PyDict_WatchEvent event,

@@ -5975,7 +5975,8 @@ PyDict_AddWatcher(PyDict_WatchCallback callback)
 {
     PyInterpreterState *interp = _PyInterpreterState_GET();
 
-    for (int i = 0; i < DICT_MAX_WATCHERS; i++) {
+    /* Start at 2, as 0 and 1 as reserved for CPython */
+    for (int i = 2; i < DICT_MAX_WATCHERS; i++) {
         if (!interp->dict_state.watchers[i]) {
             interp->dict_state.watchers[i] = callback;
             return i;

@@ -817,9 +817,10 @@ uop_optimize(
     OPT_STAT_INC(traces_created);
     char *uop_optimize = Py_GETENV("PYTHONUOPSOPTIMIZE");
     if (uop_optimize == NULL || *uop_optimize > '0') {
-        err = _Py_uop_analyze_and_optimize(frame, buffer, UOP_MAX_TRACE_LENGTH, curr_stackentries);
-        if (err < 0) {
-            return -1;
+        err = _Py_uop_analyze_and_optimize(frame, buffer,
+                                           UOP_MAX_TRACE_LENGTH, curr_stackentries, &dependencies);
+        if (err <= 0) {
+            return err;
         }
     }
     _PyExecutorObject *executor = make_executor_from_uops(buffer, &dependencies);

@@ -43,7 +43,6 @@ global_to_const(_PyUOpInstruction *inst, PyObject *obj)
     PyDictObject *dict = (PyDictObject *)obj;
     assert(dict->ma_keys->dk_kind == DICT_KEYS_UNICODE);
     PyDictUnicodeEntry *entries = DK_UNICODE_ENTRIES(dict->ma_keys);
-    assert(inst->operand < dict->ma_used);
     assert(inst->operand <= UINT16_MAX);
     PyObject *res = entries[inst->operand].me_value;
     if (res == NULL) {
@@ -59,19 +58,19 @@ global_to_const(_PyUOpInstruction *inst, PyObject *obj)
 }
 
 static int
-check_globals_version(_PyUOpInstruction *inst, PyObject *obj)
+incorrect_keys(_PyUOpInstruction *inst, PyObject *obj)
 {
     if (!PyDict_CheckExact(obj)) {
-        return -1;
+        return 1;
     }
     PyDictObject *dict = (PyDictObject *)obj;
     if (dict->ma_keys->dk_version != inst->operand) {
-        return -1;
+        return 1;
     }
     return 0;
 }
 
-static void
+static int
 remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,
                int buffer_size, _PyBloomFilter *dependencies)
 {
@@ -104,12 +103,12 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,
         int opcode = inst->opcode;
         switch(opcode) {
             case _GUARD_BUILTINS_VERSION:
-                if (check_globals_version(inst, builtins)) {
-                    continue;
+                if (incorrect_keys(inst, builtins)) {
+                    return 0;
                 }
                 if ((builtins_watched & 1) == 0) {
                     PyDict_Watch(0, builtins);
-                    builtins_watched = 1;
+                    builtins_watched |= 1;
                 }
                 if (builtins_checked & 1) {
                     buffer[pc].opcode = NOP;
@@ -121,8 +120,8 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,
                 }
                 break;
             case _GUARD_GLOBALS_VERSION:
-                if (check_globals_version(inst, globals)) {
-                    continue;
+                if (incorrect_keys(inst, globals)) {
+                    return 0;
                 }
                 if ((globals_watched & 1) == 0) {
                     PyDict_Watch(1, globals);
@@ -139,25 +138,26 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,
                 }
                 break;
             case _LOAD_GLOBAL_BUILTINS:
-                if (globals_checked & builtins_checked & globals_watched & builtins_watched & 1) {
-                    global_to_const(inst, builtins);
-                }
+                assert(globals_checked & builtins_checked & globals_watched & builtins_watched & 1);
+                global_to_const(inst, builtins);
                 break;
             case _LOAD_GLOBAL_MODULE:
-                if (globals_checked & globals_watched & 1) {
-                    global_to_const(inst, globals);
-                }
+                assert(globals_checked & globals_watched & 1);
+                global_to_const(inst, globals);
                 break;
             case _JUMP_TO_TOP:
             case _EXIT_TRACE:
-                goto done;
+                return 1;
              case _PUSH_FRAME:
              {
                  globals_checked <<= 1;
                  globals_watched <<= 1;
                  builtins_checked <<= 1;
                  builtins_watched <<= 1;
                  PyFunctionObject *func = (PyFunctionObject *)buffer[pc].operand;
+                 if (func == NULL) {
+                     return 1;
+                 }
                  assert(PyFunction_Check(func));
                  globals = func->func_globals;
                  builtins = func->func_builtins;
@@ -175,8 +175,7 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,
                  break;
         }
     }
-    done:
-        return;
+    return 0;
 }
 
 static void
@@ -266,11 +265,15 @@ _Py_uop_analyze_and_optimize(
     _PyInterpreterFrame *frame,
     _PyUOpInstruction *buffer,
     int buffer_size,
-    int curr_stacklen
+    int curr_stacklen,
+    _PyBloomFilter *dependencies
 )
 {
-    remove_globals(frame, buffer, buffer_size, dependencies);
+    int err = remove_globals(frame, buffer, buffer_size, dependencies);
+    if (err <= 0) {
+        return err;
+    }
     peephole_opt(frame, buffer, buffer_size);
     remove_unneeded_uops(buffer, buffer_size);
-    return 0;
+    return 1;
 }