Check if a speculation log is present for SpeculativeGuardMovement.

gluonhq · Oct 24, 2023 · 1f588e5 · 1f588e5
1 parent c575470
commit 1f588e5
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 12 deletions.
diff --git a/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/GraalCompilerTest.java b/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/GraalCompilerTest.java
@@ -136,6 +136,7 @@
 import jdk.graal.compiler.printer.GraalDebugHandlersFactory;
 import jdk.graal.compiler.runtime.RuntimeProvider;
 import jdk.graal.compiler.test.GraalTest;
+import jdk.graal.compiler.loop.phases.SpeculativeGuardMovementPhase;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.BeforeClass;
@@ -283,6 +284,8 @@ protected Suites createSuites(OptionValues opts) {
             ret.getHighTier().removeSubTypePhases(Speculative.class);
             ret.getMidTier().removeSubTypePhases(Speculative.class);
             ret.getLowTier().removeSubTypePhases(Speculative.class);
+            // remove after GR-49600 is resolved:
+            ret.getMidTier().replaceAllPhases(SpeculativeGuardMovementPhase.class, () -> new SpeculativeGuardMovementPhase(CanonicalizerPhase.create(), false, false));
         }
 
         ListIterator<BasePhase<? super HighTierContext>> iter = ret.getHighTier().findPhase(ConvertDeoptimizeToGuardPhase.class, true);

diff --git a/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/HashCodeTest.java b/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/HashCodeTest.java
@@ -24,13 +24,13 @@
  */
 package jdk.graal.compiler.core.test;
 
-import jdk.graal.compiler.core.phases.HighTier;
-import jdk.graal.compiler.core.phases.MidTier;
 import jdk.graal.compiler.nodes.InvokeNode;
 import jdk.graal.compiler.nodes.StructuredGraph;
 import jdk.graal.compiler.options.OptionValues;
 import jdk.graal.compiler.phases.OptimisticOptimizations;
 import jdk.graal.compiler.phases.tiers.MidTierContext;
+import jdk.graal.compiler.phases.tiers.Suites;
+
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -120,8 +120,9 @@ public void test06() {
     private StructuredGraph buildGraphAfterMidTier(String name) {
         StructuredGraph g = parseForCompile(getResolvedJavaMethod(name));
         OptionValues options = getInitialOptions();
-        new HighTier(options).apply(g, getDefaultHighTierContext());
-        new MidTier(options).apply(g, new MidTierContext(getProviders(), getTargetProvider(), OptimisticOptimizations.ALL, g.getProfilingInfo()));
+        Suites suites = createSuites(options);
+        suites.getHighTier().apply(g, getDefaultHighTierContext());
+        suites.getMidTier().apply(g, new MidTierContext(getProviders(), getTargetProvider(), OptimisticOptimizations.ALL, g.getProfilingInfo()));
         return g;
     }
 

diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/stubs/Stub.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/stubs/Stub.java
@@ -61,6 +61,8 @@
 import jdk.graal.compiler.phases.OptimisticOptimizations;
 import jdk.graal.compiler.phases.PhaseSuite;
 import jdk.graal.compiler.phases.Speculative;
+import jdk.graal.compiler.loop.phases.SpeculativeGuardMovementPhase;
+import jdk.graal.compiler.phases.common.CanonicalizerPhase;
 import jdk.graal.compiler.phases.tiers.HighTierContext;
 import jdk.graal.compiler.phases.tiers.Suites;
 import jdk.graal.compiler.printer.GraalDebugHandlersFactory;
@@ -321,6 +323,8 @@ protected Suites createSuites() {
 
         defaultSuites.getMidTier().removeSubTypePhases(Speculative.class);
         defaultSuites.getLowTier().removeSubTypePhases(Speculative.class);
+        // remove after GR-49600 is resolved:
+        defaultSuites.getMidTier().replaceAllPhases(SpeculativeGuardMovementPhase.class, () -> new SpeculativeGuardMovementPhase(CanonicalizerPhase.create(), false, false));
 
         return new Suites(emptyHighTier, defaultSuites.getMidTier(), defaultSuites.getLowTier());
     }

diff --git a/.../jdk.graal.compiler/src/jdk/graal/compiler/loop/phases/SpeculativeGuardMovementPhase.java b/.../jdk.graal.compiler/src/jdk/graal/compiler/loop/phases/SpeculativeGuardMovementPhase.java
@@ -122,8 +122,19 @@
  */
 public class SpeculativeGuardMovementPhase extends PostRunCanonicalizationPhase<MidTierContext> implements FloatingGuardPhase {
 
+    private final boolean ignoreFrequency;
+    private final boolean requireSpeculationLog;
+
     public SpeculativeGuardMovementPhase(CanonicalizerPhase canonicalizer) {
         super(canonicalizer);
+        this.ignoreFrequency = false;
+        this.requireSpeculationLog = true;
+    }
+
+    public SpeculativeGuardMovementPhase(CanonicalizerPhase canonicalizer, boolean ignoreFrequency, boolean requireSpeculationLog) {
+        super(canonicalizer);
+        this.ignoreFrequency = ignoreFrequency;
+        this.requireSpeculationLog = requireSpeculationLog;
     }
 
     @Override
@@ -158,7 +169,7 @@ protected void run(StructuredGraph graph, MidTierContext context) {
                 }
                 LoopsData loops = context.getLoopsDataProvider().getLoopsData(graph);
                 loops.detectCountedLoops();
-                iterate = performSpeculativeGuardMovement(context, graph, loops);
+                iterate = performSpeculativeGuardMovement(context, graph, loops, ignoreFrequency, requireSpeculationLog);
             }
             if (change.getNodes().isEmpty() || !iterate) {
                 break;
@@ -174,20 +185,21 @@ public void updateGraphState(GraphState graphState) {
     }
 
     public static boolean performSpeculativeGuardMovement(MidTierContext context, StructuredGraph graph, LoopsData loops) {
-        return performSpeculativeGuardMovement(context, graph, loops, null, false);
+        return performSpeculativeGuardMovement(context, graph, loops, null, false, true);
     }
 
-    public static boolean performSpeculativeGuardMovement(MidTierContext context, StructuredGraph graph, LoopsData loops, boolean ignoreFrequency) {
-        return performSpeculativeGuardMovement(context, graph, loops, null, ignoreFrequency);
+    public static boolean performSpeculativeGuardMovement(MidTierContext context, StructuredGraph graph, LoopsData loops, boolean ignoreFrequency, boolean requireSpeculationLog) {
+        return performSpeculativeGuardMovement(context, graph, loops, null, ignoreFrequency, requireSpeculationLog);
     }
 
     public static boolean performSpeculativeGuardMovement(MidTierContext context, StructuredGraph graph, LoopsData loops, NodeBitMap toProcess) {
-        return performSpeculativeGuardMovement(context, graph, loops, toProcess, false);
+        return performSpeculativeGuardMovement(context, graph, loops, toProcess, false, true);
     }
 
-    public static boolean performSpeculativeGuardMovement(MidTierContext context, StructuredGraph graph, LoopsData loops, NodeBitMap toProcess, boolean ignoreFrequency) {
+    public static boolean performSpeculativeGuardMovement(MidTierContext context, StructuredGraph graph, LoopsData loops, NodeBitMap toProcess, boolean ignoreFrequency,
+                    boolean requireSpeculationLog) {
         SpeculativeGuardMovement spec = new SpeculativeGuardMovement(loops, graph.createNodeMap(), graph, context.getProfilingInfo(), graph.getSpeculationLog(), toProcess,
-                        ignoreFrequency);
+                        ignoreFrequency, requireSpeculationLog);
         spec.run();
         return spec.iterate;
     }
@@ -204,11 +216,12 @@ private static class SpeculativeGuardMovement implements Runnable {
         private final NodeBitMap toProcess;
 
         SpeculativeGuardMovement(LoopsData loops, NodeMap<HIRBlock> earliestCache, StructuredGraph graph, ProfilingInfo profilingInfo, SpeculationLog speculationLog, NodeBitMap toProcess,
-                        boolean ignoreFrequency) {
+                        boolean ignoreFrequency, boolean requireSpeculationLog) {
             this.loops = loops;
             this.earliestCache = earliestCache;
             this.graph = graph;
             this.profilingInfo = profilingInfo;
+            GraalError.guarantee(requireSpeculationLog ? speculationLog != null : true, "Graph has no speculation log attached: %s", graph);
             this.speculationLog = speculationLog;
             this.toProcess = toProcess;
             this.ignoreFrequency = ignoreFrequency;

diff --git a/...jdk.graal.compiler/src/jdk/graal/compiler/truffle/hotspot/HotSpotTruffleCompilerImpl.java b/...jdk.graal.compiler/src/jdk/graal/compiler/truffle/hotspot/HotSpotTruffleCompilerImpl.java
@@ -61,6 +61,7 @@
 import jdk.graal.compiler.lir.asm.CompilationResultBuilderFactory;
 import jdk.graal.compiler.lir.asm.EntryPointDecorator;
 import jdk.graal.compiler.lir.phases.LIRSuites;
+import jdk.graal.compiler.loop.phases.SpeculativeGuardMovementPhase;
 import jdk.graal.compiler.nodes.StructuredGraph;
 import jdk.graal.compiler.nodes.StructuredGraph.AllowAssumptions;
 import jdk.graal.compiler.nodes.graphbuilderconf.GraphBuilderConfiguration;
@@ -74,6 +75,7 @@
 import jdk.graal.compiler.phases.PhaseSuite;
 import jdk.graal.compiler.phases.Speculative;
 import jdk.graal.compiler.phases.common.AbstractInliningPhase;
+import jdk.graal.compiler.phases.common.CanonicalizerPhase;
 import jdk.graal.compiler.phases.tiers.CompilerConfiguration;
 import jdk.graal.compiler.phases.tiers.HighTierContext;
 import jdk.graal.compiler.phases.tiers.Suites;
@@ -414,6 +416,8 @@ private static void removeSpeculativePhases(Suites suites) {
         suites.getHighTier().removeSubTypePhases(Speculative.class);
         suites.getMidTier().removeSubTypePhases(Speculative.class);
         suites.getLowTier().removeSubTypePhases(Speculative.class);
+        // remove after GR-49600 is resolved:
+        suites.getMidTier().replaceAllPhases(SpeculativeGuardMovementPhase.class, () -> new SpeculativeGuardMovementPhase(CanonicalizerPhase.create(), false, false));
     }
 
     @Override

diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/GraalSupport.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/GraalSupport.java
@@ -53,13 +53,15 @@
 import jdk.graal.compiler.lir.LIRInstructionClass;
 import jdk.graal.compiler.lir.phases.LIRPhase;
 import jdk.graal.compiler.lir.phases.LIRSuites;
+import jdk.graal.compiler.loop.phases.SpeculativeGuardMovementPhase;
 import jdk.graal.compiler.nodes.EncodedGraph;
 import jdk.graal.compiler.nodes.GraphDecoder;
 import jdk.graal.compiler.nodes.StructuredGraph;
 import jdk.graal.compiler.options.OptionValues;
 import jdk.graal.compiler.phases.BasePhase;
 import jdk.graal.compiler.phases.FloatingGuardPhase;
 import jdk.graal.compiler.phases.Speculative;
+import jdk.graal.compiler.phases.common.CanonicalizerPhase;
 import jdk.graal.compiler.phases.tiers.Suites;
 import jdk.graal.compiler.phases.util.Providers;
 import jdk.graal.compiler.serviceprovider.GraalServices;
@@ -208,6 +210,8 @@ private static Suites getWithoutSpeculative(Suites s) {
         effectiveSuites.getHighTier().removeSubTypePhases(Speculative.class);
         effectiveSuites.getMidTier().removeSubTypePhases(Speculative.class);
         effectiveSuites.getLowTier().removeSubTypePhases(Speculative.class);
+        // remove after GR-49600 is resolved:
+        effectiveSuites.getMidTier().replaceAllPhases(SpeculativeGuardMovementPhase.class, () -> new SpeculativeGuardMovementPhase(CanonicalizerPhase.create(), false, false));
         return effectiveSuites;
     }
 
@@ -219,6 +223,8 @@ private static Suites getWithExplicitExceptions(Suites s) {
         effectiveSuites.getHighTier().removeSubTypePhases(Speculative.class);
         effectiveSuites.getMidTier().removeSubTypePhases(Speculative.class);
         effectiveSuites.getLowTier().removeSubTypePhases(Speculative.class);
+        // remove after GR-49600 is resolved:
+        effectiveSuites.getMidTier().replaceAllPhases(SpeculativeGuardMovementPhase.class, () -> new SpeculativeGuardMovementPhase(CanonicalizerPhase.create(), false, false));
         return effectiveSuites;
     }
 

diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java
@@ -83,6 +83,7 @@
 import jdk.graal.compiler.java.BciBlockMapping;
 import jdk.graal.compiler.lir.phases.LIRSuites;
 import jdk.graal.compiler.loop.phases.ConvertDeoptimizeToGuardPhase;
+import jdk.graal.compiler.loop.phases.SpeculativeGuardMovementPhase;
 import jdk.graal.compiler.nodes.StructuredGraph;
 import jdk.graal.compiler.nodes.gc.BarrierSet;
 import jdk.graal.compiler.nodes.graphbuilderconf.ClassInitializationPlugin;
@@ -1639,6 +1640,8 @@ private static void removePhases(Suites suites, Class<?> c) {
         suites.getHighTier().removeSubTypePhases(c);
         suites.getMidTier().removeSubTypePhases(c);
         suites.getLowTier().removeSubTypePhases(c);
+        // remove after GR-49600 is resolved:
+        suites.getMidTier().replaceAllPhases(SpeculativeGuardMovementPhase.class, () -> new SpeculativeGuardMovementPhase(CanonicalizerPhase.create(), false, false));
     }
 
     @SuppressWarnings("unused")