From 1f588e552cc6732447987932c5afd958d18674fb Mon Sep 17 00:00:00 2001 From: Raphael Mosaner Date: Tue, 19 Sep 2023 14:17:18 +0200 Subject: [PATCH] Check if a speculation log is present for SpeculativeGuardMovement. --- .../compiler/core/test/GraalCompilerTest.java | 3 ++ .../compiler/core/test/HashCodeTest.java | 9 +++--- .../graal/compiler/hotspot/stubs/Stub.java | 4 +++ .../phases/SpeculativeGuardMovementPhase.java | 29 ++++++++++++++----- .../hotspot/HotSpotTruffleCompilerImpl.java | 4 +++ .../com/oracle/svm/graal/GraalSupport.java | 6 ++++ .../svm/hosted/NativeImageGenerator.java | 3 ++ 7 files changed, 46 insertions(+), 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 index b28fb68a3d20..446e2aa13d9e 100644 --- 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> 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 index cc823e482e95..fe55951e4176 100644 --- 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 index 698563930261..a8c93e302528 100644 --- 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/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/loop/phases/SpeculativeGuardMovementPhase.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/loop/phases/SpeculativeGuardMovementPhase.java index 14a1e889fc68..6052577de895 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/loop/phases/SpeculativeGuardMovementPhase.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/loop/phases/SpeculativeGuardMovementPhase.java @@ -122,8 +122,19 @@ */ public class SpeculativeGuardMovementPhase extends PostRunCanonicalizationPhase 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 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/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/hotspot/HotSpotTruffleCompilerImpl.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/hotspot/HotSpotTruffleCompilerImpl.java index e74ae619b2c6..d4b0a6cdf27e 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/hotspot/HotSpotTruffleCompilerImpl.java +++ b/compiler/src/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 index 51fdbcb121fd..32019015dce9 100644 --- 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,6 +53,7 @@ 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; @@ -60,6 +61,7 @@ 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 index c159659fdfc2..f5f5607b4464 100644 --- 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")