From ed50e8f07e98151c3e8e6c8d73edc6ecb8700c0e Mon Sep 17 00:00:00 2001 From: Matt D'Souza Date: Tue, 19 Mar 2024 16:14:12 -0400 Subject: [PATCH] Fix locations & get graalpytest running without crashing again --- .../bytecode_dsl/PBytecodeDSLRootNode.java | 2 +- .../nodes/frame/MaterializeFrameNode.java | 23 ++++++++++++++----- mx.graalpython/mx_graalpython.py | 17 ++++++++++---- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java index ee0b6fc541..5cea0ea4e6 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java +++ b/graalpython/com.oracle.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/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/MaterializeFrameNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/MaterializeFrameNode.java index 70aa947b02..a2d4df4cf0 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/MaterializeFrameNode.java +++ b/graalpython/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 index 1904b70c8e..874048a8eb 100644 --- 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,6 +1118,7 @@ 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) @@ -1123,8 +1126,14 @@ def graalpytest(args): 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