Fix locations & get graalpytest running without crashing again

oracle · Mar 19, 2024 · ed50e8f · ed50e8f
1 parent 41bbd27
commit ed50e8f
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 11 deletions.
diff --git a/...cle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java b/...cle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java
@@ -1481,7 +1481,7 @@ public static final class MakeSet {
         public static PSet perform(VirtualFrame frame, Object[] elements,
                         @Bind("$root") PBytecodeDSLRootNode rootNode,
                         @Bind("this") Node node,
-                        @Cached(value = "elements.length", neverDefault = true) int length,
+                        @Cached(value = "elements.length", neverDefault = false) int length,
                         @Cached SetNodes.AddNode addNode,
                         @Cached HashingCollectionNodes.SetItemNode setItemNode) {
             // TODO (GR-52217): make length a DSL constant.

diff --git a/...com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/MaterializeFrameNode.java b/...com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/MaterializeFrameNode.java
@@ -182,14 +182,25 @@ private static void processBytecodeFrame(Frame frameToMaterialize, PFrame pyFram
         if (PythonOptions.ENABLE_BYTECODE_DSL_INTERPRETER) {
             BytecodeDSLFrameInfo bytecodeDSLFrameInfo = (BytecodeDSLFrameInfo) info;
             PBytecodeDSLRootNode rootNode = bytecodeDSLFrameInfo.getRootNode();
-            if (location instanceof PBytecodeDSLRootNode) {
-                throw new AssertionError("The root node was passed as a location, but the root node is insufficient for identifying a bytecode location.");
-            } else if (location.getRootNode() != rootNode) {
+
+            if (location.getRootNode() != rootNode) {
                 throw new AssertionError("A node that did not belong to this root node was passed as a location.");
             }
-            BytecodeNode bytecodeNode = BytecodeNode.get(location);
-            pyFrame.setBci(bytecodeNode.getBytecodeLocation(frameToMaterialize, location).getBytecodeIndex());
-            pyFrame.setLocation(bytecodeNode);
+
+            if (location instanceof PBytecodeDSLRootNode) {
+                /**
+                 * Sometimes we don't have a precise location (see
+                 * {@link ReadCallerFrameNode#getFrame}). Set bci to -1 to mark the location as
+                 * unknown.
+                 */
+                pyFrame.setBci(-1);
+                pyFrame.setLocation(location);
+            } else {
+                BytecodeNode bytecodeNode = BytecodeNode.get(location);
+                assert bytecodeNode != null;
+                pyFrame.setBci(bytecodeNode.getBytecodeLocation(frameToMaterialize, location).getBytecodeIndex());
+                pyFrame.setLocation(bytecodeNode);
+            }
         } else {
             BytecodeFrameInfo bytecodeFrameInfo = (BytecodeFrameInfo) info;
             pyFrame.setBci(bytecodeFrameInfo.getBci(frameToMaterialize));

diff --git a/mx.graalpython/mx_graalpython.py b/mx.graalpython/mx_graalpython.py
@@ -752,8 +752,10 @@ def update_unittest_tags(args):
 # These test would work on JVM too, but they are prohibitively slow due to a large amount of subprocesses
 AOT_ONLY_TESTS = ["test_patched_pip.py", "test_multiprocessing_spawn.py"]
 
-# This test hangs the test runner.
-BYTECODE_DSL_INCOMPATIBLE_TESTS = ["test_pdb.py"]
+BYTECODE_DSL_INCOMPATIBLE_TESTS = [
+    "test_pdb.py", # This test hangs the test runner.
+    "test_codeobject.py", # Relies on getBytecodeNode support for continuations
+]
 
 GINSTALL_GATE_PACKAGES = {
     "numpy": "numpy",
@@ -1116,15 +1118,22 @@ def graalpytest(args):
     parser.add_argument('--python', type=str, action='store', default="", help='Run tests with custom Python binary.')
     parser.add_argument('-v', "--verbose", action="store_true", help='Verbose output.', default=True)
     parser.add_argument('-k', dest="filter", default='', help='Test pattern.')
+    parser.add_argument("--use-bytecode-dsl-interpreter", action='store_true')
     parser.add_argument('test', nargs="*", default=[], help='Test file to run (specify absolute or relative; e.g. "/path/to/test_file.py" or "cpyext/test_object.py") ')
     args, unknown_args = parser.parse_known_args(args)
 
     # ensure that the test distribution is up-to-date
     if not DISABLE_REBUILD:
         mx.command_function("build")(["--only", "com.oracle.graal.python.test"])
 
-    testfiles = _list_graalpython_unittests(args.test)
     cmd_args = []
+    excludes = []
+
+    if args.use_bytecode_dsl_interpreter:
+        cmd_args += ["--vm.Dpython.EnableBytecodeDSLInterpreter=true"]
+        excludes = BYTECODE_DSL_INCOMPATIBLE_TESTS
+
+    testfiles = _list_graalpython_unittests(args.test, exclude=excludes)
     # if we got a binary path it's most likely CPython, so don't add graalpython args
     if not args.python:
         cmd_args += ["--experimental-options=true", "--python.EnableDebuggingBuiltins"]
@@ -1133,7 +1142,7 @@ def graalpytest(args):
         mx.log(f"Executable seems to be GraalPy, prepending arguments: {gp_args}")
         cmd_args += gp_args
     # we assume that unknown args are polyglot arguments and just prepend them to the test driver
-    cmd_args += unknown_args + [_graalpytest_driver()]
+    cmd_args += unknown_args + [_graalpytest_driver(), "--report", "test_report.json"]
     if args.verbose:
         cmd_args += ["-v"]
     cmd_args += testfiles