Only one infopoint should be recorded per deopt entry

compiler/src/org.graalvm.compiler.code/src/org/graalvm/compiler/code/CompilationResult.java

@@ -35,6 +35,8 @@
 import java.util.List;
 import java.util.Objects;
 
+import org.graalvm.collections.EconomicSet;
+import org.graalvm.collections.Equivalence;
 import org.graalvm.compiler.core.common.CompilationIdentifier;
 import org.graalvm.compiler.graph.NodeSourcePosition;
 import org.graalvm.compiler.options.OptionValues;
@@ -216,6 +218,7 @@ public String toString() {
 
     private final DataSection dataSection = new DataSection();
 
+    private final EconomicSet<Infopoint> invalidDeoptimizationStates = EconomicSet.create(Equivalence.IDENTITY_WITH_SYSTEM_HASHCODE);
     private final List<Infopoint> infopoints = new ArrayList<>();
     private final List<SourceMapping> sourceMapping = new ArrayList<>();
     private final List<DataPatch> dataPatches = new ArrayList<>();
@@ -315,6 +318,7 @@ public boolean equals(Object obj) {
                 Objects.equals(this.dataSection, that.dataSection) &&
                 Objects.equals(this.exceptionHandlers, that.exceptionHandlers) &&
                 Objects.equals(this.dataPatches, that.dataPatches) &&
+                Objects.equals(this.invalidDeoptimizationStates, that.invalidDeoptimizationStates) &&
                 Objects.equals(this.infopoints, that.infopoints) &&
                 Objects.equals(this.marks,  that.marks) &&
                 Arrays.equals(this.assumptions, that.assumptions) &&
@@ -600,6 +604,14 @@ public void addInfopoint(Infopoint infopoint) {
         infopoints.add(infopoint);
     }
 
+    /**
+     * Mark that the provided infopoint cannot be used as a deoptimization entrypoint.
+     */
+    public void recordInvalidForDeoptimization(Infopoint infopoint) {
+        checkOpen();
+        invalidDeoptimizationStates.add(infopoint);
+    }
+
     public void recordSourceMapping(int startOffset, int endOffset, NodeSourcePosition sourcePosition) {
         checkOpen();
         sourceMapping.add(new SourceMapping(startOffset, endOffset, sourcePosition));
@@ -669,6 +681,10 @@ public void addAnnotation(CodeAnnotation annotation) {
         annotations.add(annotation);
     }
 
+    public boolean isValidDeoptimizationState(Infopoint infopoint) {
+        return infopoint != null && !invalidDeoptimizationStates.contains(infopoint);
+    }
+
     /**
      * @return the list of infopoints, sorted by {@link Site#pcOffset}
      */

compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/gen/DebugInfoBuilder.java

@@ -91,6 +91,7 @@ public LIRFrameState build(NodeWithState node, FrameState topState, LabelRef exc
         assert objectStates.size() == 0;
         assert pendingVirtualObjects.size() == 0;
 
+        boolean validForDeoptimization = true;
         // collect all VirtualObjectField instances:
         FrameState current = topState;
         do {
@@ -103,6 +104,7 @@ public LIRFrameState build(NodeWithState node, FrameState topState, LabelRef exc
                     }
                 }
             }
+            validForDeoptimization = validForDeoptimization && current.isValidForDeoptimization();
             current = current.outerFrameState();
         } while (current != null);
 
@@ -189,9 +191,9 @@ public LIRFrameState build(NodeWithState node, FrameState topState, LabelRef exc
         objectStates.clear();
 
         if (deoptReasonAndAction == null && deoptSpeculation == null) {
-            return new LIRFrameState(frame, virtualObjectsArray, exceptionEdge);
+            return new LIRFrameState(frame, virtualObjectsArray, exceptionEdge, validForDeoptimization);
         } else {
-            return new ImplicitLIRFrameState(frame, virtualObjectsArray, exceptionEdge, deoptReasonAndAction, deoptSpeculation);
+            return new ImplicitLIRFrameState(frame, virtualObjectsArray, exceptionEdge, deoptReasonAndAction, deoptSpeculation, validForDeoptimization);
         }
     }
 

compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/gen/NodeLIRBuilder.java

@@ -795,7 +795,7 @@ public LIRFrameState stateFor(NodeWithState deopt, FrameState state) {
 
     public LIRFrameState stateForWithExceptionEdge(NodeWithState deopt, FrameState state, LabelRef exceptionEdge) {
         if (gen.needOnlyOopMaps()) {
-            return new LIRFrameState(null, null, null);
+            return new LIRFrameState(null, null, null, false);
         }
         JavaConstant deoptReasonAndAction = null;
         JavaConstant deoptSpeculation = null;

compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/JVMCIInfopointErrorTest.java

@@ -198,7 +198,7 @@ private static LIRFrameState modifyTopFrame(LIRFrameState state, JavaValue[] val
     private static LIRFrameState modifyTopFrame(LIRFrameState state, VirtualObject[] vobj, JavaValue[] values, JavaKind[] slotKinds, int locals, int stack, int locks) {
         BytecodeFrame top = state.topFrame;
         top = new BytecodeFrame(top.caller(), top.getMethod(), top.getBCI(), top.rethrowException, top.duringCall, values, slotKinds, locals, stack, locks);
-        return new LIRFrameState(top, vobj, state.exceptionEdge);
+        return new LIRFrameState(top, vobj, state.exceptionEdge, true);
     }
 
     @Test(expected = JVMCIError.class)
@@ -314,7 +314,7 @@ public void testInvalidVirtualObjectId() {
             VirtualObject o = VirtualObject.get(obj, 5);
             o.setValues(new JavaValue[0], new JavaKind[0]);
 
-            safepoint.accept(new LIRFrameState(state.topFrame, new VirtualObject[]{o}, state.exceptionEdge));
+            safepoint.accept(new LIRFrameState(state.topFrame, new VirtualObject[]{o}, state.exceptionEdge, true));
         });
     }
 
@@ -328,7 +328,7 @@ public void testDuplicateVirtualObject() {
             VirtualObject o2 = VirtualObject.get(obj, 0);
             o2.setValues(new JavaValue[0], new JavaKind[0]);
 
-            safepoint.accept(new LIRFrameState(state.topFrame, new VirtualObject[]{o1, o2}, state.exceptionEdge));
+            safepoint.accept(new LIRFrameState(state.topFrame, new VirtualObject[]{o1, o2}, state.exceptionEdge, true));
         });
     }
 

compiler/src/org.graalvm.compiler.lir/src/org/graalvm/compiler/lir/ImplicitLIRFrameState.java

@@ -37,8 +37,8 @@ public class ImplicitLIRFrameState extends LIRFrameState {
     public final JavaConstant deoptSpeculation;
 
     public ImplicitLIRFrameState(BytecodeFrame topFrame, VirtualObject[] virtualObjects, LabelRef exceptionEdge,
-                    JavaConstant deoptReasonAndAction, JavaConstant deoptSpeculation) {
-        super(topFrame, virtualObjects, exceptionEdge);
+                    JavaConstant deoptReasonAndAction, JavaConstant deoptSpeculation, boolean validForDeoptimization) {
+        super(topFrame, virtualObjects, exceptionEdge, validForDeoptimization);
         this.deoptReasonAndAction = deoptReasonAndAction;
         this.deoptSpeculation = deoptSpeculation;
     }

compiler/src/org.graalvm.compiler.lir/src/org/graalvm/compiler/lir/LIRFrameState.java

@@ -53,7 +53,7 @@
 public class LIRFrameState {
 
     // A special marker denoting no callee save info.
-    public static final LIRFrameState NO_CALLEE_SAVE_INFO = new LIRFrameState(null, null, null);
+    public static final LIRFrameState NO_CALLEE_SAVE_INFO = new LIRFrameState(null, null, null, false);
 
     public final BytecodeFrame topFrame;
     private final VirtualObject[] virtualObjects;
@@ -62,10 +62,13 @@ public class LIRFrameState {
 
     private IndexedValueMap liveBasePointers;
 
-    public LIRFrameState(BytecodeFrame topFrame, VirtualObject[] virtualObjects, LabelRef exceptionEdge) {
+    public final boolean validForDeoptimization;
+
+    public LIRFrameState(BytecodeFrame topFrame, VirtualObject[] virtualObjects, LabelRef exceptionEdge, boolean validForDeoptimization) {
         this.topFrame = topFrame;
         this.virtualObjects = virtualObjects;
         this.exceptionEdge = exceptionEdge;
+        this.validForDeoptimization = validForDeoptimization;
     }
 
     public boolean hasDebugInfo() {

compiler/src/org.graalvm.compiler.lir/src/org/graalvm/compiler/lir/asm/CompilationResultBuilder.java

@@ -349,14 +349,30 @@ public boolean hasImplicitException(int pcOffset) {
         return false;
     }
 
+    /**
+     * Helper to mark invalid deoptimization state as needed.
+     */
+    private void recordIfInvalidForDeoptimization(LIRFrameState info, Call infopoint) {
+        if (info != null && !info.validForDeoptimization && info.hasDebugInfo()) {
+            DebugInfo debugInfo = info.debugInfo();
+            assert debugInfo != null;
+            if (debugInfo.hasFrame()) {
+                compilationResult.recordInvalidForDeoptimization(infopoint);
+            }
+        }
+    }
+
     public Call recordDirectCall(int posBefore, int posAfter, InvokeTarget callTarget, LIRFrameState info) {
         DebugInfo debugInfo = info != null ? info.debugInfo() : null;
-        return compilationResult.recordCall(posBefore, posAfter - posBefore, callTarget, debugInfo, true);
+        Call infopoint = compilationResult.recordCall(posBefore, posAfter - posBefore, callTarget, debugInfo, true);
+        recordIfInvalidForDeoptimization(info, infopoint);
+        return infopoint;
     }
 
     public void recordIndirectCall(int posBefore, int posAfter, InvokeTarget callTarget, LIRFrameState info) {
         DebugInfo debugInfo = info != null ? info.debugInfo() : null;
-        compilationResult.recordCall(posBefore, posAfter - posBefore, callTarget, debugInfo, false);
+        Call infopoint = compilationResult.recordCall(posBefore, posAfter - posBefore, callTarget, debugInfo, false);
+        recordIfInvalidForDeoptimization(info, infopoint);
     }
 
     public void recordInfopoint(int pos, LIRFrameState info, InfopointReason reason) {

compiler/src/org.graalvm.compiler.lir/src/org/graalvm/compiler/lir/gen/LIRGenerator.java

@@ -445,7 +445,7 @@ public Variable emitForeignCall(ForeignCallLinkage linkage, LIRFrameState frameS
                 state = frameState;
             } else {
                 assert needOnlyOopMaps();
-                state = new LIRFrameState(null, null, null);
+                state = new LIRFrameState(null, null, null, false);
             }
         }
 

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/FrameState.java

@@ -142,6 +142,11 @@ protected TwoSlotMarker() {
      */
     private final Bytecode code;
 
+    /**
+     * Flag to indicate whether this frame represents valid deoptimization state.
+     */
+    private boolean validForDeoptimization = true;
+
     /**
      * Narrows {@code value} to a {@code char} while ensuring the value does not change.
      */
@@ -274,6 +279,14 @@ public FrameState(FrameState outerFrameState,
         createValues(locals, stack, stackSize, pushedSlotKinds, pushedValues, locks);
     }
 
+    public boolean isValidForDeoptimization() {
+        return validForDeoptimization;
+    }
+
+    public void invalidateForDeoptimization() {
+        validForDeoptimization = false;
+    }
+
     private static int computeSize(JavaKind[] slotKinds) {
         int result = 0;
         if (slotKinds != null) {

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/CodeInfoEncoder.java

@@ -29,6 +29,8 @@
 import java.util.BitSet;
 import java.util.TreeMap;
 
+import org.graalvm.collections.EconomicSet;
+import org.graalvm.collections.Equivalence;
 import org.graalvm.compiler.code.CompilationResult;
 import org.graalvm.compiler.core.common.NumUtil;
 import org.graalvm.compiler.core.common.util.FrequencyEncoder;
@@ -198,16 +200,32 @@ public void addMethod(SharedMethod method, CompilationResult compilation, int co
             entryIP += CodeInfoDecoder.indexGranularity();
         }
 
+        EconomicSet<Integer> infopointOffsets = EconomicSet.create(Equivalence.DEFAULT);
+        EconomicSet<Long> deoptEntryBcis = EconomicSet.create(Equivalence.DEFAULT);
         /* Make entries for all calls and deoptimization entry points of the method. */
         for (Infopoint infopoint : compilation.getInfopoints()) {
             final DebugInfo debugInfo = infopoint.debugInfo;
             if (debugInfo != null) {
                 final int offset = getEntryOffset(infopoint);
                 if (offset >= 0) {
+                    boolean added = infopointOffsets.add(offset);
+                    if (!added) {
+                        throw VMError.shouldNotReachHere("Encoding two infopoints at same offset. Conflicting infopoint: " + infopoint);
+                    }
                     IPData entry = makeEntry(offset + compilationOffset);
                     assert entry.referenceMap == null && entry.frameData == null;
                     entry.referenceMap = (ReferenceMapEncoder.Input) debugInfo.getReferenceMap();
-                    entry.frameData = frameInfoEncoder.addDebugInfo(method, infopoint, totalFrameSize);
+                    entry.frameData = frameInfoEncoder.addDebugInfo(method, compilation, infopoint, totalFrameSize);
+                    if (entry.frameData != null && entry.frameData.frame.isDeoptEntry) {
+                        BytecodeFrame frame = debugInfo.frame();
+                        long encodedBci = FrameInfoEncoder.encodeBci(frame.getBCI(), frame.duringCall, frame.rethrowException);
+                        added = deoptEntryBcis.add(encodedBci);
+                        if (!added) {
+                            String errorMessage = String.format("Encoding two deopt entries at same encoded bci: %s (bci %s)\nmethod: %s", encodedBci, FrameInfoDecoder.readableBci(encodedBci),
+                                            method);
+                            throw VMError.shouldNotReachHere(errorMessage);
+                        }
+                    }
                 }
             }
         }

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/FrameInfoEncoder.java

@@ -38,6 +38,7 @@
 
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.collections.Equivalence;
+import org.graalvm.compiler.code.CompilationResult;
 import org.graalvm.compiler.core.common.LIRKind;
 import org.graalvm.compiler.core.common.NumUtil;
 import org.graalvm.compiler.core.common.util.FrequencyEncoder;
@@ -85,6 +86,15 @@
 public class FrameInfoEncoder {
 
     public abstract static class Customization {
+
+        /**
+         * Hook for when a frame is registered for encoding.
+         */
+        @SuppressWarnings("unused")
+        protected void recordFrame(ResolvedJavaMethod method, Infopoint infopoint, boolean isDeoptEntry) {
+
+        }
+
         /**
          * Returns true if the method's deoptimization target should be saved within the debugInfo.
          */
@@ -95,17 +105,19 @@ public abstract static class Customization {
          *
          * @param method The method that contains the debugInfo.
          * @param infopoint The infopoint whose debugInfo that is considered for inclusion.
+         * @param isDeoptEntry whether this infopoint is tied to a deoptimization entrypoint.
          */
-        protected abstract boolean includeLocalValues(ResolvedJavaMethod method, Infopoint infopoint);
+        protected abstract boolean includeLocalValues(ResolvedJavaMethod method, Infopoint infopoint, boolean isDeoptEntry);
 
         /**
          * Returns true if the given debugInfo is a valid entry point for deoptimization (and not
          * just frame information for the purpose of debugging).
          *
          * @param method The method that contains the debugInfo.
+         * @param compilation method compilation this infopoint is within.
          * @param infopoint The infopoint whose debugInfo that is considered for inclusion.
          */
-        protected abstract boolean isDeoptEntry(ResolvedJavaMethod method, Infopoint infopoint);
+        protected abstract boolean isDeoptEntry(ResolvedJavaMethod method, CompilationResult compilation, Infopoint infopoint);
 
         /**
          * Fills the FrameInfoQueryResult.source* fields.
@@ -444,8 +456,10 @@ protected FrameInfoEncoder(Customization customization, Encoders encoders) {
         this.frameMetadata = new CompressedFrameInfoEncodingMetadata();
     }
 
-    protected FrameData addDebugInfo(ResolvedJavaMethod method, Infopoint infopoint, int totalFrameSize) {
-        final boolean includeLocalValues = customization.includeLocalValues(method, infopoint);
+    protected FrameData addDebugInfo(ResolvedJavaMethod method, CompilationResult compilation, Infopoint infopoint, int totalFrameSize) {
+        final boolean isDeoptEntry = customization.isDeoptEntry(method, compilation, infopoint);
+        customization.recordFrame(method, infopoint, isDeoptEntry);
+        final boolean includeLocalValues = customization.includeLocalValues(method, infopoint, isDeoptEntry);
         final boolean encodeSourceReferences = FrameInfoDecoder.encodeSourceReferences();
         final boolean useCompressedEncoding = SubstrateOptions.UseCompressedFrameEncodings.getValue() && !includeLocalValues;
         if (!includeLocalValues && !encodeSourceReferences) {
@@ -457,7 +471,7 @@ protected FrameData addDebugInfo(ResolvedJavaMethod method, Infopoint infopoint,
         data.debugInfo = debugInfo;
         data.totalFrameSize = totalFrameSize;
         data.virtualObjects = new ValueInfo[countVirtualObjects(debugInfo)][];
-        data.frame = addFrame(data, debugInfo.frame(), customization.isDeoptEntry(method, infopoint), includeLocalValues);
+        data.frame = addFrame(data, debugInfo.frame(), isDeoptEntry, includeLocalValues);
 
         if (encodeSourceReferences) {
             List<CompressedFrameData> frameSlice = useCompressedEncoding ? new ArrayList<>() : null;

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/deopt/Deoptimizer.java

@@ -810,11 +810,20 @@ private VirtualFrame constructTargetFrame(CodeInfoQueryResult targetInfo, FrameI
         targetContentSize += FrameAccess.returnAddressSize();
 
         /* The source and target bytecode frame must match (as they stem from the same BCI). */
-        assert sourceFrame.getNumLocals() == targetFrame.getNumLocals();
-        assert sourceFrame.getNumStack() == targetFrame.getNumStack();
-        assert sourceFrame.getNumLocks() == targetFrame.getNumLocks();
-        assert targetFrame.getVirtualObjects().length == 0;
-        assert sourceFrame.getValueInfos().length >= targetFrame.getValueInfos().length;
+        boolean compatibleState = sourceFrame.getNumLocals() == targetFrame.getNumLocals() &&
+                        sourceFrame.getNumStack() == targetFrame.getNumStack() &&
+                        sourceFrame.getNumLocks() == targetFrame.getNumLocks() &&
+                        targetFrame.getVirtualObjects().length == 0 &&
+                        sourceFrame.getValueInfos().length >= targetFrame.getValueInfos().length;
+        if (!compatibleState) {
+            String message = "Deoptimization is not possible.\n" +
+                            String.format("Target Frame: numLocals-%s, numStack-%s, numLocks-%s, getValueInfos length-%s, virtual objects length-%s\n", targetFrame.getNumLocals(),
+                                            targetFrame.getNumStack(), targetFrame.getNumLocks(), targetFrame.getValueInfos().length, targetFrame.getVirtualObjects().length) +
+                            String.format("Source Frame: numLocals-%s, numStack-%s, numLocks-%s, getValueInfos length-%s\n", sourceFrame.getNumLocals(), sourceFrame.getNumStack(),
+                                            sourceFrame.getNumLocks(), sourceFrame.getValueInfos().length);
+            throw VMError.shouldNotReachHere(message);
+        }
+
         int numValues = targetFrame.getValueInfos().length;
 
         /*

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/feature/InternalFeature.java

@@ -80,6 +80,7 @@ default void registerGraphBuilderPlugins(Providers providers, Plugins plugins, P
      * @param providers Providers that the lowering can use.
      * @param lowerings The lowering provider registry to add to.
      * @param hosted True if registering for ahead-of-time compilation, false if registering for
+     *            runtime compilation.
      */
     default void registerLowerings(RuntimeConfiguration runtimeConfig, OptionValues options, Providers providers,
                     Map<Class<? extends Node>, NodeLoweringProvider<?>> lowerings, boolean hosted) {
@@ -93,6 +94,7 @@ default void registerLowerings(RuntimeConfiguration runtimeConfig, OptionValues
      * @param snippetReflection Snippet reflection providers.
      * @param suites The Graal compilation suites to add to.
      * @param hosted True if registering for ahead-of-time compilation, false if registering for
+     *            runtime compilation.
      */
     default void registerGraalPhases(Providers providers, SnippetReflectionProvider snippetReflection, Suites suites, boolean hosted) {
     }