Merge remote-tracking branch 'upstream/master'

gluonhq · Oct 26, 2023 · 87011d1 · 87011d1
2 parents 27c42f0 + f9ab576
commit 87011d1
Show file tree

Hide file tree

Showing 90 changed files with 1,043 additions and 869 deletions.
diff --git a/ci/ci_common/ci-check.libsonnet b/ci/ci_common/ci-check.libsonnet
@@ -3,7 +3,7 @@ local std_get = (import 'common-utils.libsonnet').std_get;
 {
   # check that all non [gate, ondemand] entries have notify_emails or notify_groups defined
   local missing_notify(builds) = {
-    [x.name]: std_get(x, "defined_in") for x in builds if !std.objectHas(x, "notify_emails") && !std.objectHasAll(x, "notify_groups") && (std.member(x.targets, "daily") || std.member(x.targets, "weekly") || std.member(x.targets, "monthly"))
+    [x.name]: std_get(x, "defined_in") for x in builds if !std.objectHas(x, "notify_emails") && !std.objectHasAll(x, "notify_groups") && std.length(std.setInter(std.set(x.targets), std.set(["daily", "weekly", "monthly", "post-merge", "opt-post-merge"]))) != 0
   },
 
   verify_ci(builds)::

diff --git a/ci/ci_common/common-utils.libsonnet b/ci/ci_common/common-utils.libsonnet
@@ -26,6 +26,11 @@
   else
     build,
 
+  # Adds a 'defined_in' key to all jobs in the list.
+  # Due to the nature of std.thisFile, the file name has to be explicitly passed.
+  # Usage: add_defined_in(builds, std.thisFile)
+  add_defined_in(builds, file):: [{ defined_in: file } + b for b in builds],
+
   # Returns true if `str` contains `needle` as a substring.
   contains(str, needle):: std.findSubstr(needle, str) != [],
 

diff --git a/ci_includes/publish-javadoc.jsonnet b/ci_includes/publish-javadoc.jsonnet
@@ -62,5 +62,5 @@
     common.post_merge + linux_amd64 + common.labsjdk21 + javadoc_publisher,
   ],
   // adds a "defined_in" field to all builds mentioning the location of this current file
-  builds:: [{ defined_in: std.thisFile } + b for b in all_builds]
+  builds:: utils.add_defined_in(all_builds, std.thisFile),
 }
diff --git a/compiler/ci/ci_common/benchmark-builders.jsonnet b/compiler/ci/ci_common/benchmark-builders.jsonnet
@@ -1,5 +1,6 @@
 {
   local c = (import '../../../ci/ci_common/common.jsonnet'),
+  local utils = (import '../../../ci/ci_common/common-utils.libsonnet'),
   local bc = (import '../../../ci/ci_common/bench-common.libsonnet'),
   local cc = (import 'compiler-common.libsonnet'),
   local bench = (import 'benchmark-suites.libsonnet'),
@@ -91,5 +92,5 @@
   local all_builds = main_builds + weekly_amd64_forks_builds + weekly_aarch64_forks_builds + profiling_builds + avx_builds + zgc_builds + zgc_avx_builds + aarch64_builds + no_tiered_builds + no_profile_info_builds,
   local filtered_builds = [b for b in all_builds if b.is_jdk_supported(b.jdk_version) && b.is_arch_supported(b.arch)],
   // adds a "defined_in" field to all builds mentioning the location of this current file
-  builds:: [{ defined_in: std.thisFile } + b for b in filtered_builds]
+  builds:: utils.add_defined_in(filtered_builds, std.thisFile),
 }
diff --git a/compiler/ci/ci_includes/baseline-benchmarks.jsonnet b/compiler/ci/ci_includes/baseline-benchmarks.jsonnet
@@ -1,5 +1,6 @@
 {
   local c = (import '../../../ci/ci_common/common.jsonnet'),
+  local utils = (import '../../../ci/ci_common/common-utils.libsonnet'),
   local bc = (import '../../../ci/ci_common/bench-common.libsonnet'),
   local cc = (import '../ci_common/compiler-common.libsonnet'),
   local bench = (import '../ci_common/benchmark-suites.libsonnet'),
@@ -79,5 +80,5 @@
   local filtered_builds = [b for b in all_builds if b.is_jdk_supported(b.jdk_version) && b.is_arch_supported(b.arch)],
 
   // adds a "defined_in" field to all builds mentioning the location of this current file
-  builds:: [{ defined_in: std.thisFile } + b for b in filtered_builds]
+  builds:: utils.add_defined_in(filtered_builds, std.thisFile),
 }
diff --git a/...dk.graal.compiler.test/src/jdk/graal/compiler/core/test/ConditionalEliminationTest11.java b/...dk.graal.compiler.test/src/jdk/graal/compiler/core/test/ConditionalEliminationTest11.java
@@ -126,7 +126,7 @@ public void test5() {
 
     public static int test6Snippet(int a) {
         if ((a & 8) != 0) {
-            GraalDirectives.deoptimize();
+            GraalDirectives.deoptimizeAndInvalidate();
         }
         if ((a & 15) != 15) {
             GraalDirectives.deoptimize();
@@ -135,8 +135,11 @@ public static int test6Snippet(int a) {
     }
 
     public static int reference6Snippet(int a) {
+        /*
+         * first guard needs higher priority than second guard, otherwise code folds to second guard
+         */
         if ((a & 8) != 0) {
-            GraalDirectives.deoptimize();
+            GraalDirectives.deoptimizeAndInvalidate();
         }
         GraalDirectives.deoptimize();
         return 0;

diff --git a/...jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/ConditionalEliminationTest9.java b/...jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/ConditionalEliminationTest9.java
@@ -27,8 +27,6 @@
 import jdk.graal.compiler.phases.common.ConditionalEliminationPhase;
 import org.junit.Test;
 
-import jdk.graal.compiler.api.directives.GraalDirectives;
-
 /**
  * Collection of tests for {@link ConditionalEliminationPhase} including those that triggered bugs
  * in this phase.
@@ -40,7 +38,7 @@ public class ConditionalEliminationTest9 extends ConditionalEliminationTestBase
     @SuppressWarnings("all")
     public static int referenceSnippet(int a) {
         if (a == 0) {
-            GraalDirectives.deoptimize();
+            return 1;
         }
         return 0;
     }
@@ -54,10 +52,10 @@ public void test1() {
     public static int test1Snippet(int a) {
         if (a == 0) {
             if (a == 0) {
-                GraalDirectives.deoptimize();
+                return 1;
             }
             if (a == 0) {
-                GraalDirectives.deoptimize();
+                return 2;
             }
         }
         return 0;

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/.../jdk.graal.compiler.test/src/jdk/graal/compiler/hotspot/test/Base64SubstitutionsTest.java b/.../jdk.graal.compiler.test/src/jdk/graal/compiler/hotspot/test/Base64SubstitutionsTest.java
@@ -38,9 +38,6 @@
 import java.util.Set;
 import java.util.function.Supplier;
 import java.util.stream.Stream;
-
-import jdk.graal.compiler.core.common.GraalOptions;
-import jdk.graal.compiler.options.OptionValues;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -141,20 +138,6 @@ public void testEncodeByteArray1() {
         }
     }
 
-    /**
-     * The test at
-     * https://github.com/openjdk/jdk/blob/f2922682688a40529df269e1551246ac8da5d7ee/src/java.base/share/classes/java/util/Base64.java#L878
-     * is converted to a guard and (too eagerly) floated outside the loop, leading to deopt even
-     * when the input byte array has a correct ending.
-     *
-     * GR-48430.
-     */
-    private static OptionValues workaroundTooEagerDeopt() {
-        return new OptionValues(getInitialOptions(),
-                        GraalOptions.OptConvertDeoptsToGuards, false,
-                        GraalOptions.SpeculativeGuardMovement, false);
-    }
-
     /**
      * Tests {@link Encoder#encode(byte[], byte[])}.
      */
@@ -297,15 +280,14 @@ public void testDecodeByteArray1() {
      */
     @Test
     public void testDecodeByteArray2() {
-        OptionValues options = workaroundTooEagerDeopt();
         ResolvedJavaMethod m = getResolvedJavaMethod(Decoder.class, "decode", byte[].class, byte[].class);
         for (DecoderTestCase tc : getDecoders()) {
             for (int i = 0; i < PLAIN_TEXT_BYTES.length; i++) {
                 byte[] srcBytes = tc.encoded[i];
                 // JDK-8273108: Test for output buffer overrun
                 byte[] suffix = {0, (byte) 167};
                 ByteArraySupplier bas = new ByteArraySupplier(srcBytes.length, suffix);
-                test(options, m, tc.decoder, srcBytes, bas);
+                test(m, tc.decoder, srcBytes, bas);
                 Assert.assertEquals(bas.supplied.size(), 2);
                 byte[] expect = Arrays.copyOfRange(bas.supplied.get(0), 0, srcBytes.length);
                 byte[] actual = Arrays.copyOfRange(bas.supplied.get(1), 0, srcBytes.length);
@@ -328,12 +310,11 @@ public static byte[] decodeByteBufferSnippet(Decoder decoder, ByteBuffer srcBuf)
      */
     @Test
     public void testDecodeByteBuffer() {
-        OptionValues options = workaroundTooEagerDeopt();
         for (DecoderTestCase tc : getDecoders()) {
             for (int i = 0; i < PLAIN_TEXT_BYTES.length; i++) {
                 byte[] srcBytes = tc.encoded[i];
                 ByteBuffer srcBuf = ByteBuffer.wrap(srcBytes);
-                test(options, "decodeByteBufferSnippet", tc.decoder, srcBuf);
+                test("decodeByteBufferSnippet", tc.decoder, srcBuf);
             }
         }
     }

diff --git a/...jdk.graal.compiler.test/src/jdk/graal/compiler/nodes/test/IfNodeCanonicalizationTest.java b/...jdk.graal.compiler.test/src/jdk/graal/compiler/nodes/test/IfNodeCanonicalizationTest.java
@@ -24,7 +24,6 @@
  */
 package jdk.graal.compiler.nodes.test;
 
-import jdk.graal.compiler.api.directives.GraalDirectives;
 import jdk.graal.compiler.core.test.GraalCompilerTest;
 import jdk.graal.compiler.graph.Node;
 import jdk.graal.compiler.loop.phases.ConvertDeoptimizeToGuardPhase;
@@ -123,10 +122,10 @@ public void test4() {
     public boolean testSnippet4(int a, int[] limit) {
         int l = limit.length;
         if (a < 0) {
-            GraalDirectives.deoptimize();
+            return false;
         }
         if (a >= l) {
-            GraalDirectives.deoptimize();
+            return false;
         }
         return true;
     }
@@ -140,10 +139,10 @@ public void test5() {
     public boolean testSnippet5(int a, int[] limit) {
         int l = limit.length;
         if (a >= l) {
-            GraalDirectives.deoptimize();
+            return false;
         }
         if (a < 0) {
-            GraalDirectives.deoptimize();
+            return false;
         }
         return true;
     }

diff --git a/...dk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/TransferToInterpreterTest.java b/...dk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/TransferToInterpreterTest.java
@@ -90,7 +90,8 @@ public Object execute(VirtualFrame frame) {
             int x = (int) frame.getArguments()[0];
             if (x == 0) {
                 CompilerDirectives.transferToInterpreter();
-            } else {
+            }
+            if (x == 1) {
                 CompilerDirectives.transferToInterpreterAndInvalidate();
             }
             return null;

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/ConvertDeoptimizeToGuardPhase.java b/.../jdk.graal.compiler/src/jdk/graal/compiler/loop/phases/ConvertDeoptimizeToGuardPhase.java
@@ -110,9 +110,6 @@ protected void run(final StructuredGraph graph, final CoreProviders context) {
 
         for (DeoptimizeNode d : graph.getNodes(DeoptimizeNode.TYPE)) {
             assert d.isAlive();
-            if (!d.canFloat()) {
-                continue;
-            }
             try (DebugCloseable closable = d.withNodeSourcePosition()) {
                 propagateFixed(d, d, context, lazyLoops);
             }

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/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/DeoptimizeNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/DeoptimizeNode.java
@@ -138,7 +138,8 @@ public boolean canFloat() {
      * high and would be executed under the wrong conditions.
      */
     public static boolean canFloat(DeoptimizationReason reason, DeoptimizationAction action) {
-        return action != DeoptimizationAction.None && reason != DeoptimizationReason.Unresolved;
+        return action != DeoptimizationAction.None && reason != DeoptimizationReason.Unresolved && reason != DeoptimizationReason.NotCompiledExceptionHandler &&
+                        reason != DeoptimizationReason.UnreachedCode;
     }
 
     @NodeIntrinsic