[GR-40038][GR-39992] Improve compilation in bytecode interpreter

msimacek · msimacek · commit de882cfe6e6c · 2022-08-08T14:50:49.000Z
PullRequest: graalpython/2360
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/PythonBuiltinClassType.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/PythonBuiltinClassType.java
@@ -382,7 +382,7 @@ public enum PythonBuiltinClassType implements TruffleObject {
     Capsule("PyCapsule"),
 
     // A marker for @Builtin that is not a class. Must always come last.
-    nil(null);
+    nil("nil");
 
     private static class Flags {
 
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/GraalPythonModuleBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/GraalPythonModuleBuiltins.java
@@ -71,6 +71,7 @@
 import org.graalvm.polyglot.io.ByteSequence;
 
 import com.oracle.graal.python.PythonLanguage;
+import com.oracle.graal.python.annotations.ArgumentClinic;
 import com.oracle.graal.python.builtins.Builtin;
 import com.oracle.graal.python.builtins.CoreFunctions;
 import com.oracle.graal.python.builtins.Python3Core;
@@ -112,8 +113,10 @@
 import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
 import com.oracle.graal.python.nodes.function.PythonBuiltinNode;
 import com.oracle.graal.python.nodes.function.builtins.PythonBinaryBuiltinNode;
+import com.oracle.graal.python.nodes.function.builtins.PythonBinaryClinicBuiltinNode;
 import com.oracle.graal.python.nodes.function.builtins.PythonTernaryBuiltinNode;
 import com.oracle.graal.python.nodes.function.builtins.PythonUnaryBuiltinNode;
+import com.oracle.graal.python.nodes.function.builtins.clinic.ArgumentClinicProvider;
 import com.oracle.graal.python.nodes.object.GetClassNode;
 import com.oracle.graal.python.nodes.statement.AbstractImportNode;
 import com.oracle.graal.python.nodes.subscript.GetItemNode;
@@ -650,33 +653,40 @@ protected static boolean isType(Object obj, Env env, InteropLibrary lib) {
 
     }
 
-    @Builtin(name = "dis", minNumOfPositionalArgs = 1, doc = "Helper to disassemble code objects if running with the bytecode interpreter")
+    @Builtin(name = "dis", minNumOfPositionalArgs = 1, parameterNames = {"obj", "quickened"}, doc = "Helper to disassemble code objects if running with the bytecode interpreter")
+    @ArgumentClinic(name = "quickened", conversion = ArgumentClinic.ClinicConversion.Boolean, defaultValue = "false")
     @GenerateNodeFactory
-    public abstract static class BCIDisNode extends PythonUnaryBuiltinNode {
+    public abstract static class BCIDisNode extends PythonBinaryClinicBuiltinNode {
         @Specialization
-        Object doMethod(PMethod method) {
+        Object doMethod(PMethod method, boolean quickened) {
             final Object function = method.getFunction();
             if (function instanceof PFunction) {
-                return doFunction((PFunction) function);
+                return doFunction((PFunction) function, quickened);
             }
             return PNone.NONE;
         }
 
         @Specialization
-        Object doFunction(PFunction function) {
-            return doCode(function.getCode());
+        Object doFunction(PFunction function, boolean quickened) {
+            return doCode(function.getCode(), quickened);
         }
 
         @Specialization
         @TruffleBoundary
-        Object doCode(PCode code) {
-            return toTruffleStringUncached(code.toDisassembledString());
+        Object doCode(PCode code, boolean quickened) {
+            return toTruffleStringUncached(code.toDisassembledString(quickened));
         }
 
-        @Specialization(guards = {"!isCode(value)", "!isPFunction(value)", "!isMethod(value)"})
-        Object doObject(@SuppressWarnings("unused") Object value) {
+        @Fallback
+        @SuppressWarnings("unused")
+        Object doObject(Object value, Object quickened) {
             return PNone.NONE;
         }
+
+        @Override
+        protected ArgumentClinicProvider getArgumentClinic() {
+            return GraalPythonModuleBuiltinsClinicProviders.BCIDisNodeClinicProviderGen.INSTANCE;
+        }
     }
 
     @Builtin(name = "super", minNumOfPositionalArgs = 1, doc = "Returns HostAdapter instance of the object or None")
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/code/PCode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/code/PCode.java
@@ -746,16 +746,26 @@ public boolean hasSourceLocation(@Exclusive @Cached GilNode gil) {
     @TruffleBoundary
     public String toString() {
         String codeName = this.getName() == null ? "None" : this.getName().toJavaStringUncached();
-        String codeFilename = this.getFilename() == null ? "None" : this.getFilename().toJavaStringUncached();
+        /*
+         * This might be called without an active context (i.e. when dumping graphs), we cannot use
+         * getFilename
+         */
+        String codeFilename = getSourceSectionFileName(rootNodeForExtraction(getRootNode()).getSourceSection());
+        if (codeFilename == null) {
+            codeFilename = "None";
+        }
         int codeFirstLineNo = this.getFirstLineNo() == 0 ? -1 : this.getFirstLineNo();
         return String.format("<code object %s, file \"%s\", line %d>", codeName, codeFilename, codeFirstLineNo);
     }
 
     @TruffleBoundary
-    public String toDisassembledString() {
+    public String toDisassembledString(boolean quickened) {
         final RootNode rootNode = getRootCallTarget().getRootNode();
         if (rootNode instanceof PBytecodeRootNode) {
             CodeUnit code = ((PBytecodeRootNode) rootNode).getCodeUnit();
+            if (quickened) {
+                return code.toString(((PBytecodeRootNode) rootNode).getBytecode());
+            }
             return code.toString();
         }
         return J_EMPTY_STRING;
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/CompilationUnit.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/CompilationUnit.java
@@ -206,6 +206,7 @@ public CodeUnit assemble() {
         }
         int[] boxingMetric = new int[varCount];
         byte[] shouldUnboxVariable = new byte[varCount];
+        Arrays.fill(shouldUnboxVariable, (byte) 0xff);
 
         SortedSet<int[]> finishedExceptionHandlerRanges = new TreeSet<>(Comparator.comparingInt(a -> a[0]));
 
@@ -243,7 +244,6 @@ public CodeUnit assemble() {
                 if (i.opcode == OpCodes.STORE_FAST) {
                     variableStores.get(i.arg).add(i);
                 } else if (i.opcode == OpCodes.LOAD_FAST) {
-                    shouldUnboxVariable[i.arg] |= i.quickenOutput;
                     boxingMetric[i.arg] += i.quickenOutput != 0 ? 1 : -1;
                 }
                 i.bci = buf.size();
@@ -296,14 +296,24 @@ public CodeUnit assemble() {
                 }
             }
         }
-        for (int i = 0; i < varCount; i++) {
-            List<Instruction> stores = variableStores.get(i);
-            finishedGeneralizeVarsMap[i] = new int[stores.size()];
-            for (int j = 0; j < stores.size(); j++) {
-                finishedGeneralizeVarsMap[i][j] = stores.get(j).bci;
-            }
-            if (boxingMetric[i] <= 0) {
-                shouldUnboxVariable[i] = 0;
+        if (!scope.isGenerator()) {
+            /*
+             * We do an optimization in the interpreter that we don't unbox variables that would
+             * mostly get boxed again. This helps for interpreter performance, but for compiled code
+             * we have to unbox all variables otherwise the compiler is not always able to prove the
+             * variable was initialized. In generators, transferring the variables between the two
+             * modes of usage (boxed vs unboxed) would be too complex, so we skip the optimization
+             * there and unbox all variables.
+             */
+            for (int i = 0; i < varCount; i++) {
+                List<Instruction> stores = variableStores.get(i);
+                finishedGeneralizeVarsMap[i] = new int[stores.size()];
+                for (int j = 0; j < stores.size(); j++) {
+                    finishedGeneralizeVarsMap[i][j] = stores.get(j).bci;
+                }
+                if (boxingMetric[i] <= 0) {
+                    shouldUnboxVariable[i] = 0;
+                }
             }
         }
         return new CodeUnit(toTruffleStringUncached(name), toTruffleStringUncached(qualName),
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/OpCodes.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/OpCodes.java
@@ -681,8 +681,10 @@ public enum OpCodes {
     LOAD_FAST_B(LOAD_FAST, 0, QuickeningTypes.BOOLEAN, LOAD_FAST_B_BOX),
     STORE_FAST_O(STORE_FAST, QuickeningTypes.OBJECT, 0),
     STORE_FAST_UNBOX_I(STORE_FAST, QuickeningTypes.OBJECT, 0),
+    STORE_FAST_BOXED_I(STORE_FAST, QuickeningTypes.OBJECT, 0),
     STORE_FAST_I(STORE_FAST, QuickeningTypes.INT, 0),
     STORE_FAST_UNBOX_B(STORE_FAST, QuickeningTypes.OBJECT, 0),
+    STORE_FAST_BOXED_B(STORE_FAST, QuickeningTypes.OBJECT, 0),
     STORE_FAST_B(STORE_FAST, QuickeningTypes.BOOLEAN, 0),
     UNARY_OP_O_O(UNARY_OP, QuickeningTypes.OBJECT, QuickeningTypes.OBJECT),
     UNARY_OP_I_O(UNARY_OP, QuickeningTypes.INT, QuickeningTypes.OBJECT),
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/QuickeningTypes.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/QuickeningTypes.java
@@ -40,10 +40,43 @@
  */
 package com.oracle.graal.python.compiler;
 
+import com.oracle.truffle.api.CompilerDirectives;
+import com.oracle.truffle.api.frame.FrameSlotKind;
+
 public abstract class QuickeningTypes {
     public static final byte OBJECT = 1;
     public static final byte INT = 2;
     public static final byte LONG = 4;
     public static final byte DOUBLE = 8;
     public static final byte BOOLEAN = 16;
+
+    public static byte fromFrameSlotTag(byte tag) {
+        if (tag == FrameSlotKind.Object.tag) {
+            return OBJECT;
+        } else if (tag == FrameSlotKind.Int.tag) {
+            return INT;
+        } else if (tag == FrameSlotKind.Long.tag) {
+            return LONG;
+        } else if (tag == FrameSlotKind.Double.tag) {
+            return DOUBLE;
+        } else if (tag == FrameSlotKind.Boolean.tag) {
+            return BOOLEAN;
+        } else {
+            throw CompilerDirectives.shouldNotReachHere("Unknown stack item type");
+        }
+    }
+
+    public static byte fromObjectType(Object object) {
+        if (object instanceof Integer) {
+            return INT;
+        } else if (object instanceof Long) {
+            return LONG;
+        } else if (object instanceof Double) {
+            return DOUBLE;
+        } else if (object instanceof Boolean) {
+            return BOOLEAN;
+        } else {
+            return OBJECT;
+        }
+    }
 }
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/ForIterINode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/ForIterINode.java
@@ -58,7 +58,7 @@
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.frame.Frame;
 import com.oracle.truffle.api.frame.VirtualFrame;
-import com.oracle.truffle.api.profiles.ConditionProfile;
+import com.oracle.truffle.api.profiles.LoopConditionProfile;
 
 /**
  * Obtains the next value of an iterator. When the iterator is exhausted it returns {@code null}. It
@@ -70,7 +70,7 @@ public abstract class ForIterINode extends PNodeWithContext {
 
     @Specialization
     boolean doIntRange(VirtualFrame frame, PIntRangeIterator iterator, int stackTop,
-                    @Cached("createCountingProfile()") ConditionProfile conditionProfile) {
+                    @Cached("createCountingProfile()") LoopConditionProfile conditionProfile) {
         if (conditionProfile.profile(iterator.hasNextInt())) {
             frame.setInt(stackTop, iterator.nextInt());
             return true;
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/ForIterONode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/ForIterONode.java
@@ -56,7 +56,7 @@
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.frame.Frame;
 import com.oracle.truffle.api.frame.VirtualFrame;
-import com.oracle.truffle.api.profiles.ConditionProfile;
+import com.oracle.truffle.api.profiles.LoopConditionProfile;
 
 /**
  * Obtains the next value of an iterator. When the iterator is exhausted it returns {@code null}. It
@@ -68,7 +68,7 @@ public abstract class ForIterONode extends PNodeWithContext {
 
     @Specialization
     boolean doIntRange(VirtualFrame frame, PIntRangeIterator iterator, int stackTop,
-                    @Cached("createCountingProfile()") ConditionProfile conditionProfile) {
+                    @Cached("createCountingProfile()") LoopConditionProfile conditionProfile) {
         if (conditionProfile.profile(iterator.hasNextInt())) {
             frame.setObject(stackTop, iterator.nextInt());
             return true;
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/GetSendValueNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/GetSendValueNode.java
@@ -0,0 +1,77 @@
+/*
+ * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * The Universal Permissive License (UPL), Version 1.0
+ *
+ * Subject to the condition set forth below, permission is hereby granted to any
+ * person obtaining a copy of this software, associated documentation and/or
+ * data (collectively the "Software"), free of charge and under any and all
+ * copyright rights in the Software, and any and all patent rights owned or
+ * freely licensable by each licensor hereunder covering either (i) the
+ * unmodified Software as contributed to or provided by such licensor, or (ii)
+ * the Larger Works (as defined below), to deal in both
+ *
+ * (a) the Software, and
+ *
+ * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
+ * one is included with the Software each a "Larger Work" to which the Software
+ * is contributed by such licensors),
+ *
+ * without restriction, including without limitation the rights to copy, create
+ * derivative works of, display, perform, and distribute the Software and make,
+ * use, sell, offer for sale, import, export, have made, and have sold the
+ * Software and the Larger Work(s), and to sublicense the foregoing rights on
+ * either these or other terms.
+ *
+ * This license is subject to the following condition:
+ *
+ * The above copyright notice and either this complete permission notice or at a
+ * minimum a reference to the UPL must be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+package com.oracle.graal.python.nodes.bytecode;
+
+import com.oracle.graal.python.builtins.objects.PNone;
+import com.oracle.graal.python.builtins.objects.generator.ThrowData;
+import com.oracle.graal.python.nodes.PNodeWithContext;
+import com.oracle.graal.python.runtime.exception.PException;
+import com.oracle.truffle.api.dsl.Fallback;
+import com.oracle.truffle.api.dsl.GenerateUncached;
+import com.oracle.truffle.api.dsl.Specialization;
+
+@GenerateUncached
+public abstract class GetSendValueNode extends PNodeWithContext {
+    public abstract Object execute(Object specialArgument);
+
+    @Specialization(guards = "specialArgument == null")
+    Object doNext(@SuppressWarnings("unused") Object specialArgument) {
+        return PNone.NONE;
+    }
+
+    @Specialization
+    Object doThrow(ThrowData throwData) {
+        throw PException.fromObject(throwData.pythonException, this, throwData.withJavaStacktrace);
+    }
+
+    @Fallback
+    Object doSend(Object obj) {
+        return obj;
+    }
+
+    public static GetSendValueNode create() {
+        return GetSendValueNodeGen.create();
+    }
+
+    public static GetSendValueNode getUncached() {
+        return GetSendValueNodeGen.getUncached();
+    }
+}
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/OSRInterpreterState.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/OSRInterpreterState.java
@@ -42,10 +42,8 @@
 
 final class OSRInterpreterState {
     public final int stackTop;
-    public final int loopEndBci;
 
-    public OSRInterpreterState(int stackTop, int loopEndBci) {
+    public OSRInterpreterState(int stackTop) {
         this.stackTop = stackTop;
-        this.loopEndBci = loopEndBci;
     }
 }
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/PBytecodeGeneratorRootNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/PBytecodeGeneratorRootNode.java
@@ -139,7 +139,7 @@ private void copyStackSlotsIntoVirtualFrame(MaterializedFrame generatorFrame, Vi
     public Object executeOSR(VirtualFrame osrFrame, int target, Object interpreterStateObject) {
         OSRInterpreterState interpreterState = (OSRInterpreterState) interpreterStateObject;
         MaterializedFrame generatorFrame = PArguments.getGeneratorFrame(osrFrame);
-        return rootNode.executeFromBci(osrFrame, generatorFrame, this, target, interpreterState.stackTop, interpreterState.loopEndBci);
+        return rootNode.executeFromBci(osrFrame, generatorFrame, this, target, interpreterState.stackTop);
     }
 
     @Override
@@ -157,7 +157,7 @@ public Object execute(VirtualFrame frame) {
          */
         copyStackSlotsIntoVirtualFrame(generatorFrame, frame);
         try {
-            return rootNode.executeFromBci(frame, generatorFrame, this, resumeBci, resumeStackTop, Integer.MAX_VALUE);
+            return rootNode.executeFromBci(frame, generatorFrame, this, resumeBci, resumeStackTop);
         } finally {
             calleeContext.exit(frame, this);
         }
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/PBytecodeRootNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/PBytecodeRootNode.java

Original file line number	Diff line number	Diff line change
`@@ -42,10 +42,8 @@`
`42`	`42`
`43`	`43`	`final class OSRInterpreterState {`
`44`	`44`	`public final int stackTop;`
`45`		`- public final int loopEndBci;`
`46`	`45`
`47`		`- public OSRInterpreterState(int stackTop, int loopEndBci) {`
	`46`	`+ public OSRInterpreterState(int stackTop) {`
`48`	`47`	`this.stackTop = stackTop;`
`49`		`- this.loopEndBci = loopEndBci;`
`50`	`48`	`}`
`51`	`49`	`}`