]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-104584: Clean up and fix uops tests and fix crash (#106492)
authorGuido van Rossum <guido@python.org>
Thu, 6 Jul 2023 22:45:56 +0000 (15:45 -0700)
committerGitHub <noreply@github.com>
Thu, 6 Jul 2023 22:45:56 +0000 (15:45 -0700)
The uops test wasn't testing anything by default,
and was failing when run with -Xuops.

Made the two executor-related context managers global,
so TestUops can use them (notably `with temporary_optimizer(opt)`).

Made clear_executor() a little more thorough.

Fixed a crash upon finalizing a uop optimizer,
by adding a `tp_dealloc` handler.

Lib/test/test_capi/test_misc.py
Python/optimizer.c

index de9f00a9e5fb484c4ecb7ca509f5d0b87420ac82..8d597e79902557c6d8b4052b4a674b18dcfb033a 100644 (file)
@@ -2343,23 +2343,26 @@ class Test_Pep523API(unittest.TestCase):
         names = ["func", "outer", "outer", "inner", "inner", "outer", "inner"]
         self.do_test(func, names)
 
-class TestOptimizerAPI(unittest.TestCase):
 
-    @contextlib.contextmanager
-    def temporary_optimizer(self, opt):
-        _testinternalcapi.set_optimizer(opt)
-        try:
-            yield
-        finally:
-            _testinternalcapi.set_optimizer(None)
+@contextlib.contextmanager
+def temporary_optimizer(opt):
+    _testinternalcapi.set_optimizer(opt)
+    try:
+        yield
+    finally:
+        _testinternalcapi.set_optimizer(None)
 
-    @contextlib.contextmanager
-    def clear_executors(self, func):
-        try:
-            yield
-        finally:
-            #Clear executors
-            func.__code__ = func.__code__.replace()
+@contextlib.contextmanager
+def clear_executors(func):
+    # Clear executors in func before and after running a block
+    func.__code__ = func.__code__.replace()
+    try:
+        yield
+    finally:
+        func.__code__ = func.__code__.replace()
+
+
+class TestOptimizerAPI(unittest.TestCase):
 
     def test_get_set_optimizer(self):
         self.assertEqual(_testinternalcapi.get_optimizer(), None)
@@ -2381,9 +2384,9 @@ class TestOptimizerAPI(unittest.TestCase):
 
         for repeat in range(5):
             opt = _testinternalcapi.get_counter_optimizer()
-            with self.temporary_optimizer(opt):
+            with temporary_optimizer(opt):
                 self.assertEqual(opt.get_count(), 0)
-                with self.clear_executors(loop):
+                with clear_executors(loop):
                     loop()
                 self.assertEqual(opt.get_count(), 1000)
 
@@ -2409,7 +2412,7 @@ class TestOptimizerAPI(unittest.TestCase):
         long_loop = ns['long_loop']
 
         opt = _testinternalcapi.get_counter_optimizer()
-        with self.temporary_optimizer(opt):
+        with temporary_optimizer(opt):
             self.assertEqual(opt.get_count(), 0)
             long_loop()
             self.assertEqual(opt.get_count(), 10)
@@ -2418,24 +2421,27 @@ class TestOptimizerAPI(unittest.TestCase):
 class TestUops(unittest.TestCase):
 
     def test_basic_loop(self):
-
         def testfunc(x):
             i = 0
             while i < x:
                 i += 1
 
-        testfunc(1000)
+        opt = _testinternalcapi.get_uop_optimizer()
+
+        with temporary_optimizer(opt):
+            testfunc(1000)
 
         ex = None
-        for offset in range(0, 100, 2):
+        for offset in range(0, len(testfunc.__code__.co_code), 2):
             try:
                 ex = _testinternalcapi.get_executor(testfunc.__code__, offset)
                 break
             except ValueError:
                 pass
-        if ex is None:
-            return
-        self.assertIn("SAVE_IP", str(ex))
+        self.assertIsNotNone(ex)
+        uops = {opname for opname, _ in ex}
+        self.assertIn("SAVE_IP", uops)
+        self.assertIn("LOAD_FAST", uops)
 
 
 if __name__ == "__main__":
index 2c1be61f8d614e86ef6595e1db51484f6575c4a5..d2fdca59fe46c8e54c0c3f9136bc06f7e5baf26d 100644 (file)
@@ -532,7 +532,7 @@ uop_optimize(
         return trace_length;
     }
     OBJECT_STAT_INC(optimization_traces_created);
-    _PyUOpExecutorObject *executor = (_PyUOpExecutorObject *)_PyObject_New(&UOpExecutor_Type);
+    _PyUOpExecutorObject *executor = PyObject_New(_PyUOpExecutorObject, &UOpExecutor_Type);
     if (executor == NULL) {
         return -1;
     }
@@ -542,18 +542,24 @@ uop_optimize(
     return 1;
 }
 
+static void
+uop_opt_dealloc(PyObject *self) {
+    PyObject_Free(self);
+}
+
 static PyTypeObject UOpOptimizer_Type = {
     PyVarObject_HEAD_INIT(&PyType_Type, 0)
     .tp_name = "uop_optimizer",
     .tp_basicsize = sizeof(_PyOptimizerObject),
     .tp_itemsize = 0,
     .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_DISALLOW_INSTANTIATION,
+    .tp_dealloc = uop_opt_dealloc,
 };
 
 PyObject *
 PyUnstable_Optimizer_NewUOpOptimizer(void)
 {
-    _PyOptimizerObject *opt = (_PyOptimizerObject *)_PyObject_New(&UOpOptimizer_Type);
+    _PyOptimizerObject *opt = PyObject_New(_PyOptimizerObject, &UOpOptimizer_Type);
     if (opt == NULL) {
         return NULL;
     }