Prevent deopts after exitting OSR

Author: msimacek

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/OSRInterpreterState.java

@@ -0,0 +1,51 @@
+/*
+ * 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;
+
+final class OSRInterpreterState {
+    public final int stackTop;
+    public final int loopEndBci;
+
+    public OSRInterpreterState(int stackTop, int loopEndBci) {
+        this.stackTop = stackTop;
+        this.loopEndBci = loopEndBci;
+    }
+}

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/PBytecodeGeneratorRootNode.java

@@ -163,12 +163,12 @@ private void profileFrameSlots(MaterializedFrame generatorFrame) {
     }
 
     @Override
-    public Object executeOSR(VirtualFrame osrFrame, int target, Object interpreterState) {
-        Integer osrStackTop = (Integer) interpreterState;
+    public Object executeOSR(VirtualFrame osrFrame, int target, Object interpreterStateObject) {
+        OSRInterpreterState interpreterState = (OSRInterpreterState) interpreterStateObject;
         MaterializedFrame generatorFrame = PArguments.getGeneratorFrame(osrFrame);
         copyStackSlotsIntoVirtualFrame(generatorFrame, osrFrame);
-        copyOSRStackRemainderIntoVirtualFrame(generatorFrame, osrFrame, osrStackTop);
-        return rootNode.executeFromBci(osrFrame, generatorFrame, osrFrame, this, target, osrStackTop);
+        copyOSRStackRemainderIntoVirtualFrame(generatorFrame, osrFrame, interpreterState.stackTop);
+        return rootNode.executeFromBci(osrFrame, generatorFrame, osrFrame, this, target, interpreterState.stackTop, interpreterState.loopEndBci);
     }
 
     @ExplodeLoop
@@ -205,7 +205,7 @@ public Object execute(VirtualFrame frame) {
             stackFrame = frame;
         }
         try {
-            return rootNode.executeFromBci(frame, generatorFrame, stackFrame, this, resumeBci, resumeStackTop);
+            return rootNode.executeFromBci(frame, generatorFrame, stackFrame, this, resumeBci, resumeStackTop, Integer.MAX_VALUE);
         } finally {
             calleeContext.exit(frame, this);
         }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/PBytecodeRootNode.java

@@ -712,7 +712,7 @@ public Object execute(VirtualFrame virtualFrame) {
                 copyArgsAndCells(virtualFrame, virtualFrame.getArguments());
             }
 
-            return executeFromBci(virtualFrame, virtualFrame, virtualFrame, this, 0, getInitialStackTop());
+            return executeFromBci(virtualFrame, virtualFrame, virtualFrame, this, 0, getInitialStackTop(), Integer.MAX_VALUE);
         } finally {
             calleeContext.exit(virtualFrame, this);
         }
@@ -731,14 +731,25 @@ public Frame restoreParentFrameFromArguments(Object[] arguments) {
     }
 
     @Override
-    public Object executeOSR(VirtualFrame osrFrame, int target, Object interpreterState) {
-        return executeFromBci(osrFrame, osrFrame, osrFrame, this, target, (Integer) interpreterState);
+    public Object executeOSR(VirtualFrame osrFrame, int target, Object interpreterStateObject) {
+        OSRInterpreterState interpreterState = (OSRInterpreterState) interpreterStateObject;
+        return executeFromBci(osrFrame, osrFrame, osrFrame, this, target, interpreterState.stackTop, interpreterState.loopEndBci);
+    }
+
+    private static final class OSRContinuation {
+        public final int bci;
+        public final int stackTop;
+
+        private OSRContinuation(int bci, int stackTop) {
+            this.bci = bci;
+            this.stackTop = stackTop;
+        }
     }
 
     @BytecodeInterpreterSwitch
     @ExplodeLoop(kind = ExplodeLoop.LoopExplosionKind.MERGE_EXPLODE)
     @SuppressWarnings("fallthrough")
-    Object executeFromBci(VirtualFrame virtualFrame, Frame localFrame, Frame stackFrame, BytecodeOSRNode osrNode, int initialBci, int initialStackTop) {
+    Object executeFromBci(VirtualFrame virtualFrame, Frame localFrame, Frame stackFrame, BytecodeOSRNode osrNode, int initialBci, int initialStackTop, int loopEndBci) {
         Object globals = PArguments.getGlobals(virtualFrame);
         Object locals = PArguments.getSpecialArgument(virtualFrame);
 
@@ -787,6 +798,22 @@ Object executeFromBci(VirtualFrame virtualFrame, Frame localFrame, Frame stackFr
             CompilerAsserts.partialEvaluationConstant(bci);
             CompilerAsserts.partialEvaluationConstant(stackTop);
 
+            if (CompilerDirectives.inCompiledCode() && bci > loopEndBci) {
+                /*
+                 * This means we're in OSR and we just jumped out of the OSR compiled loop. We want
+                 * to return to the caller to continue in interpreter again otherwise we would most
+                 * likely deopt on the next instruction. The caller handles the special return value
+                 * in JUMP_BACKWARD. In generators, we need to additionally copy the stack items
+                 * back to the generator frame.
+                 */
+                if (localFrame != stackFrame) {
+                    copyStackSlotsToGeneratorFrame(stackFrame, localFrame, stackTop);
+                    // Clear slots that were popped (if any)
+                    clearFrameSlots(localFrame, stackTop + 1, initialStackTop);
+                }
+                return new OSRContinuation(bci, stackTop);
+            }
+
             try {
                 switch (bc) {
                     case OpCodesConstants.LOAD_NONE:
@@ -1185,12 +1212,22 @@ Object executeFromBci(VirtualFrame virtualFrame, Frame localFrame, Frame stackFr
                              * will get mixed up. To retain such state, put it into the frame
                              * instead.
                              */
-                            Object osrResult = BytecodeOSRNode.tryOSR(osrNode, bci, stackTop, null, virtualFrame);
+                            Object osrResult = BytecodeOSRNode.tryOSR(osrNode, bci, new OSRInterpreterState(stackTop, beginBci), null, virtualFrame);
                             if (osrResult != null) {
-                                if (CompilerDirectives.hasNextTier() && loopCount[0] > 0) {
-                                    LoopNode.reportLoopCount(this, loopCount[0]);
+                                if (osrResult instanceof OSRContinuation) {
+                                    // We should continue executing in interpreter after the loop
+                                    OSRContinuation continuation = (OSRContinuation) osrResult;
+                                    bci = continuation.bci;
+                                    stackTop = continuation.stackTop;
+                                    oparg = 0;
+                                    continue;
+                                } else {
+                                    // We reached a return/yield
+                                    if (CompilerDirectives.hasNextTier() && loopCount[0] > 0) {
+                                        LoopNode.reportLoopCount(this, loopCount[0]);
+                                    }
+                                    return osrResult;
                                 }
-                                return osrResult;
                             }
                         }
                         oparg = 0;