address reviewer feedback

Author: teshull

substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/RuntimeCompilationFeature.java

@@ -697,13 +697,14 @@ private void recordFailed(AnalysisMethod method) {
         public boolean validateGraph(PointsToAnalysis bb, StructuredGraph graph) {
             PointsToAnalysisMethod aMethod = (PointsToAnalysisMethod) graph.method();
             MultiMethod.MultiMethodKey multiMethodKey = aMethod.getMultiMethodKey();
-            Supplier<Boolean> graphInvalidator = DeoptimizationUtils.createGraphInvalidator(graph, multiMethodKey == RUNTIME_COMPILED_METHOD);
+            Supplier<Boolean> graphChecker = DeoptimizationUtils.createGraphChecker(graph,
+                            multiMethodKey == RUNTIME_COMPILED_METHOD ? DeoptimizationUtils.RUNTIME_COMPILATION_INVALID_NODES : DeoptimizationUtils.AOT_COMPILATION_INVALID_NODES);
             if (multiMethodKey != ORIGINAL_METHOD) {
-                if (graphInvalidator.get()) {
+                if (!graphChecker.get()) {
                     recordFailed(aMethod);
                     return false;
                 }
-            } else if (DeoptimizationUtils.canDeoptForTesting(aMethod, false, graphInvalidator)) {
+            } else if (DeoptimizationUtils.canDeoptForTesting(aMethod, false, graphChecker)) {
                 DeoptimizationUtils.registerDeoptEntriesForDeoptTesting(bb, graph, aMethod);
                 return true;
             }
@@ -882,7 +883,7 @@ protected AbstractPolicyScope openCalleeScope(AbstractPolicyScope outer, Analysi
                  * Once the accumulative policy is activated, we cannot return to the trivial
                  * policy.
                  */
-                return inliningUtils.createAccumulativeInlineScope(accOuter, method, DeoptimizationUtils.runtimeCompilationInvalidNodes);
+                return inliningUtils.createAccumulativeInlineScope(accOuter, method, DeoptimizationUtils.RUNTIME_COMPILATION_INVALID_NODES);
             }
 
             assert outer == null || outer instanceof RuntimeCompilationAlwaysInlineScope : "unexpected outer scope: " + outer;
@@ -902,7 +903,7 @@ protected AbstractPolicyScope openCalleeScope(AbstractPolicyScope outer, Analysi
                 return new RuntimeCompilationAlwaysInlineScope(inliningDepth);
             } else {
                 // start with a new accumulative inline scope
-                return inliningUtils.createAccumulativeInlineScope(null, method, DeoptimizationUtils.runtimeCompilationInvalidNodes);
+                return inliningUtils.createAccumulativeInlineScope(null, method, DeoptimizationUtils.RUNTIME_COMPILATION_INVALID_NODES);
             }
         }
     }
@@ -1147,7 +1148,7 @@ public boolean processNode(AnalysisMetaAccess metaAccess, AnalysisMethod method,
         /*
          * Inline as long as an invalid node has not been seen.
          */
-        return !DeoptimizationUtils.runtimeCompilationInvalidNodes.test(node);
+        return !DeoptimizationUtils.RUNTIME_COMPILATION_INVALID_NODES.test(node);
     }
 
     @Override
@@ -1158,6 +1159,6 @@ public boolean processNonInlinedInvoke(CoreProviders providers, CallTargetNode n
 
     @Override
     public String toString() {
-        return "AlwaysInlineScope";
+        return "RuntimeCompilationAlwaysInlineScope";
     }
 }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/DeoptimizationUtils.java

@@ -127,7 +127,7 @@ static void insertDeoptTests(HostedMethod method, StructuredGraph graph) {
      *
      * Note this should only be called within CompileQueue#parseAheadOfTimeCompiledMethods
      */
-    public static boolean canDeoptForTesting(AnalysisMethod method, boolean deoptimizeAll, Supplier<Boolean> graphInvalidator) {
+    public static boolean canDeoptForTesting(AnalysisMethod method, boolean deoptimizeAll, Supplier<Boolean> graphChecker) {
         if (SubstrateCompilationDirectives.singleton().isRegisteredForDeoptTesting(method)) {
             return true;
         }
@@ -146,7 +146,7 @@ public static boolean canDeoptForTesting(AnalysisMethod method, boolean deoptimi
             return false;
         }
 
-        if (graphInvalidator.get()) {
+        if (!graphChecker.get()) {
             return false;
         }
 
@@ -291,7 +291,7 @@ static boolean verifyDeoptTarget(HostedMethod method, StructuredGraph graph, Com
         /*
          * No deopt targets can have a StackValueNode in the graph.
          */
-        assert !createGraphInvalidator(graph, false).get() : "Invalid nodes in deopt target: " + graph;
+        assert createGraphChecker(graph, AOT_COMPILATION_INVALID_NODES).get() : "Invalid nodes in deopt target: " + graph;
 
         for (Infopoint infopoint : result.getInfopoints()) {
             if (infopoint.debugInfo != null) {
@@ -500,29 +500,28 @@ public static Collection<ResolvedJavaMethod> registerDeoptEntries(StructuredGrap
      * We also do not allow class initialization at run time to ensure the partial evaluator does
      * not constant fold uninitialized fields.
      */
-    public static final NodePredicate runtimeCompilationInvalidNodes = n -> NodePredicates.isA(StackValueNode.class).or(NodePredicates.isA(EnsureClassInitializedNode.class)).test(n);
+    public static final NodePredicate RUNTIME_COMPILATION_INVALID_NODES = n -> NodePredicates.isA(StackValueNode.class).or(NodePredicates.isA(EnsureClassInitializedNode.class)).test(n);
 
-    private static final NodePredicate stackOnly = n -> NodePredicates.isA(StackValueNode.class).test(n);
+    public static final NodePredicate AOT_COMPILATION_INVALID_NODES = n -> NodePredicates.isA(StackValueNode.class).test(n);
 
     /**
-     * @return Supplier which returns true if the graph contains invalid nodes.
+     * @return Supplier which returns true if the graph does not violate the checks.
      */
-    public static Supplier<Boolean> createGraphInvalidator(StructuredGraph graph, boolean isRuntimeCompiledMethod) {
+    public static Supplier<Boolean> createGraphChecker(StructuredGraph graph, NodePredicate invalidNodes) {
         return () -> {
             if (!graph.method().getDeclaringClass().isInitialized()) {
                 /*
                  * All types which are used at run time should build-time initialized. This ensures
                  * the partial evaluator does not constant fold uninitialized fields.
                  */
-                return true;
+                return false;
             }
 
-            NodePredicate predicate = isRuntimeCompiledMethod ? runtimeCompilationInvalidNodes : stackOnly;
-            if (graph.getNodes().filter(predicate).isNotEmpty()) {
-                return true;
+            if (graph.getNodes().filter(invalidNodes).isNotEmpty()) {
+                return false;
             }
 
-            return false;
+            return true;
         };
     }
 }