[GR-44071] Safepoint removal: add option to control loop end safepoin…

…t removal.

PullRequest: graal/13735

compiler/src/org.graalvm.compiler.loop.phases/src/org/graalvm/compiler/loop/phases/LoopSafepointEliminationPhase.java

@@ -40,13 +40,23 @@
 import org.graalvm.compiler.nodes.loop.LoopEx;
 import org.graalvm.compiler.nodes.loop.LoopsData;
 import org.graalvm.compiler.nodes.spi.CoreProviders;
+import org.graalvm.compiler.options.Option;
+import org.graalvm.compiler.options.OptionKey;
+import org.graalvm.compiler.options.OptionType;
 import org.graalvm.compiler.phases.BasePhase;
 import org.graalvm.compiler.phases.tiers.MidTierContext;
 
 import jdk.vm.ci.meta.ResolvedJavaMethod;
 
 public class LoopSafepointEliminationPhase extends BasePhase<MidTierContext> {
 
+    public static class Options {
+        //@formatter:off
+        @Option(help = "Remove safepoints on counted loop ends.", type = OptionType.Expert)
+        public static final OptionKey<Boolean> RemoveLoopSafepoints = new OptionKey<>(true);
+        //@formatter:on
+    }
+
     private static final long IntegerRangeDistance = Math.abs((long) Integer.MAX_VALUE - (long) Integer.MIN_VALUE);
 
     /**
@@ -114,35 +124,38 @@ public Optional<NotApplicable> notApplicableTo(GraphState graphState) {
     @Override
     protected final void run(StructuredGraph graph, MidTierContext context) {
         LoopsData loops = context.getLoopsDataProvider().getLoopsData(graph);
-        loops.detectCountedLoops();
-        for (LoopEx loop : loops.countedLoops()) {
-            if (loop.loop().getChildren().isEmpty() && (loop.loopBegin().isPreLoop() || loop.loopBegin().isPostLoop() || loopIsIn32BitRange(loop) || loop.loopBegin().isStripMinedInner())) {
-                boolean hasSafepoint = false;
-                for (LoopEndNode loopEnd : loop.loopBegin().loopEnds()) {
-                    hasSafepoint |= loopEnd.canSafepoint();
-                }
-                if (hasSafepoint) {
-                    if (!loop.counted().counterNeverOverflows()) {
-                        // Counter can overflow, need to create a guard.
-                        boolean allowsLoopLimitChecks = context.getOptimisticOptimizations().useLoopLimitChecks(graph.getOptions());
-                        boolean allowsFloatingGuards = graph.getGuardsStage().allowsFloatingGuards();
-                        if (allowsLoopLimitChecks && allowsFloatingGuards) {
-                            loop.counted().createOverFlowGuard();
+        if (Options.RemoveLoopSafepoints.getValue(graph.getOptions())) {
+            loops.detectCountedLoops();
+            for (LoopEx loop : loops.countedLoops()) {
+                if (loop.loop().getChildren().isEmpty() && (loop.loopBegin().isPreLoop() || loop.loopBegin().isPostLoop() || loopIsIn32BitRange(loop) || loop.loopBegin().isStripMinedInner())) {
+                    boolean hasSafepoint = false;
+                    for (LoopEndNode loopEnd : loop.loopBegin().loopEnds()) {
+                        hasSafepoint |= loopEnd.canSafepoint();
+                    }
+                    if (hasSafepoint) {
+                        if (!loop.counted().counterNeverOverflows()) {
+                            // Counter can overflow, need to create a guard.
+                            boolean allowsLoopLimitChecks = context.getOptimisticOptimizations().useLoopLimitChecks(graph.getOptions());
+                            boolean allowsFloatingGuards = graph.getGuardsStage().allowsFloatingGuards();
+                            if (allowsLoopLimitChecks && allowsFloatingGuards) {
+                                loop.counted().createOverFlowGuard();
+                            } else {
+                                // Cannot disable this safepoint, because the loop could overflow.
+                                continue;
+                            }
+                        }
+                        loop.loopBegin().disableSafepoint();
+                        if (loop.loopBegin().isStripMinedInner()) {
+                            // graal strip mined this loop, trust the heuristics and remove the
+                            // inner
+                            // loop safepoint
+                            loop.loopBegin().disableGuestSafepoint();
                         } else {
-                            // Cannot disable this safepoint, because the loop could overflow.
-                            continue;
+                            // let the shape of the loop decide whether a guest safepoint is needed
+                            onSafepointDisabledLoopBegin(loop);
                         }
+                        graph.getOptimizationLog().report(LoopSafepointEliminationPhase.class, "SafepointElimination", loop.loopBegin());
                     }
-                    loop.loopBegin().disableSafepoint();
-                    if (loop.loopBegin().isStripMinedInner()) {
-                        // graal strip mined this loop, trust the heuristics and remove the inner
-                        // loop safepoint
-                        loop.loopBegin().disableGuestSafepoint();
-                    } else {
-                        // let the shape of the loop decide whether a guest safepoint is needed
-                        onSafepointDisabledLoopBegin(loop);
-                    }
-                    graph.getOptimizationLog().report(LoopSafepointEliminationPhase.class, "SafepointElimination", loop.loopBegin());
                 }
             }
         }

compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/util/OptimizationUtility.java

@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package org.graalvm.compiler.phases.common.util;
+
+import org.graalvm.compiler.nodes.StructuredGraph;
+import org.graalvm.compiler.options.Option;
+import org.graalvm.compiler.options.OptionKey;
+
+public class OptimizationUtility {
+
+    public static class Options {
+        @Option(help = "Minimal self time for a compilation unit to be considered hot globally.")//
+        public static final OptionKey<Double> HotCodeMinSelfTime = new OptionKey<>(0.001);
+
+    }
+
+    public static <X> X chooseAdaptiveBudgetFactor(StructuredGraph graph, OptionKey<X> coldOption, OptionKey<X> hotOption) {
+        return hotGlobalSelfTime(graph) ? hotOption.getValue(graph.getOptions()) : coldOption.getValue(graph.getOptions());
+    }
+
+    public static <X> X chooseAdaptiveBudgetFactor(StructuredGraph graph, X coldValue, OptionKey<X> hotOption) {
+        return hotGlobalSelfTime(graph) ? hotOption.getValue(graph.getOptions()) : coldValue;
+    }
+
+    public static <X> X chooseAdaptiveBudgetFactor(StructuredGraph graph, X coldValue, X hotValue) {
+        return hotGlobalSelfTime(graph) ? hotValue : coldValue;
+    }
+
+    /**
+     * Determine if the given graph should be considered "hot" for additional optimization purposes.
+     * We define "hot" by inspecting its self time with respect to overall execution time. This is a
+     * pure heuristical value.
+     */
+    public static boolean hotGlobalSelfTime(StructuredGraph graph) {
+        return graph.getSelfTimePercent() > Options.HotCodeMinSelfTime.getValue(graph.getOptions());
+    }
+}

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

@@ -477,6 +477,15 @@ protected LIRSuites createDeoptTargetLIRSuites() {
     protected void modifyRegularSuites(@SuppressWarnings("unused") Suites suites) {
     }
 
+    /**
+     * Get suites for the compilation of {@code graph}. Parameter {@code suites} can be used to
+     * create new suites, they must not be modified directly as this code is called concurrently by
+     * multiple threads. Rather implementors can copy suites and modify them.
+     */
+    protected Suites createSuitesForRegularCompile(@SuppressWarnings("unused") StructuredGraph graph, Suites originalSuites) {
+        return originalSuites;
+    }
+
     protected PhaseSuite<HighTierContext> afterParseCanonicalization() {
         PhaseSuite<HighTierContext> phaseSuite = new PhaseSuite<>();
         phaseSuite.appendPhase(new ImplicitAssertionsPhase());
@@ -1219,7 +1228,7 @@ private CompilationResult defaultCompileFunction(DebugContext debug, HostedMetho
                 method.compilationInfo.numDuringCallEntryPoints = graph.getNodes(MethodCallTargetNode.TYPE).snapshot().stream().map(MethodCallTargetNode::invoke).filter(
                                 invoke -> method.compilationInfo.isDeoptEntry(invoke.bci(), true, false)).count();
 
-                Suites suites = method.isDeoptTarget() ? deoptTargetSuites : regularSuites;
+                Suites suites = method.isDeoptTarget() ? deoptTargetSuites : createSuitesForRegularCompile(graph, regularSuites);
                 LIRSuites lirSuites = method.isDeoptTarget() ? deoptTargetLIRSuites : regularLIRSuites;
 
                 CompilationResult result = backend.newCompilationResult(compilationIdentifier, method.getQualifiedName());