[GR-66037] Don't use TruffleConstantFieldProvider for caching in CachingPEGraphDecoder.

PullRequest: graal/21183

Author: jchalou

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/replacements/test/PEGraphDecoderTest.java

@@ -232,7 +232,7 @@ private StructuredGraph test(String methodName, EconomicMap<ResolvedJavaMethod,
             registerPlugins(graphBuilderConfig.getPlugins().getInvocationPlugins());
             targetGraph = new StructuredGraph.Builder(debug.getOptions(), debug, AllowAssumptions.YES).method(testMethod).build();
             GraphBuilderPhase.Instance instance = new TestGraphBuilderPhase.Instance(getProviders(), graphBuilderConfig, OptimisticOptimizations.NONE, null);
-            CachingPEGraphDecoder decoder = new CachingPEGraphDecoder(getTarget().arch, targetGraph, getProviders(), graphBuilderConfig,
+            CachingPEGraphDecoder decoder = new CachingPEGraphDecoder(getTarget().arch, targetGraph, getProviders(), getProviders(), graphBuilderConfig,
                             null, null, new InlineInvokePlugin[]{new InlineAll()}, null, null, null, null, null, graphCache, () -> null, instance, false, false, true);
 
             decoder.decode(testMethod);

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/DeoptLoopDetectionTest.java

@@ -34,6 +34,7 @@
 
 import org.graalvm.polyglot.Context;
 import org.junit.After;
+import org.junit.Assert;
 import org.junit.Assume;
 import org.junit.Before;
 import org.junit.Test;
@@ -150,6 +151,32 @@ public Object execute(VirtualFrame frame) {
         }, 1);
     }
 
+    static class StaticAssumptionRootNode extends BaseRootNode {
+        @CompilationFinal static volatile Assumption assumption = Assumption.create();
+
+        @Override
+        public Object execute(VirtualFrame frame) {
+            if (!assumption.isValid()) {
+                CompilerDirectives.transferToInterpreterAndInvalidate();
+            }
+            return 0;
+        }
+    }
+
+    @Test
+    public void testStaticAssumptionNoDeopt() {
+        boolean[] firstExecution = new boolean[]{true};
+        AssertionError expectedError = Assert.assertThrows(AssertionError.class, () -> assertDeoptLoop(new StaticAssumptionRootNode(), "staticAssumptionNoDeopt", (target) -> {
+            target.call();
+            if (firstExecution[0]) {
+                StaticAssumptionRootNode.assumption.invalidate();
+                StaticAssumptionRootNode.assumption = Assumption.create();
+                firstExecution[0] = false;
+            }
+        }, 1));
+        Assert.assertEquals("No deopt loop detected after " + MAX_EXECUTIONS + " executions", expectedError.getMessage());
+    }
+
     @Test
     public void testLocalDeoptWithChangedCode() {
         assertDeoptLoop(new BaseRootNode() {

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/CachingPEGraphDecoder.java

@@ -66,7 +66,7 @@ public class CachingPEGraphDecoder extends PEGraphDecoder {
 
     private static final TimerKey BuildGraphTimer = DebugContext.timer("PartialEvaluation-GraphBuilding");
 
-    protected final Providers providers;
+    protected final Providers graphCacheProviders;
     protected final GraphBuilderConfiguration graphBuilderConfig;
     private final EconomicMap<ResolvedJavaMethod, EncodedGraph> persistentGraphCache;
     private final EconomicMap<ResolvedJavaMethod, EncodedGraph> localGraphCache;
@@ -84,7 +84,8 @@ public class CachingPEGraphDecoder extends PEGraphDecoder {
      */
     public CachingPEGraphDecoder(Architecture architecture,
                     StructuredGraph graph,
-                    Providers providers,
+                    Providers graphCacheProviders,
+                    Providers decodingProviders,
                     GraphBuilderConfiguration graphBuilderConfig,
                     LoopExplosionPlugin loopExplosionPlugin,
                     InvocationPlugins invocationPlugins,
@@ -100,13 +101,13 @@ public CachingPEGraphDecoder(Architecture architecture,
                     boolean allowAssumptionsDuringParsing,
                     boolean needsExplicitException,
                     boolean forceLink) {
-        super(architecture, graph, providers, loopExplosionPlugin,
+        super(architecture, graph, decodingProviders, loopExplosionPlugin,
                         invocationPlugins, inlineInvokePlugins, parameterPlugin, nodePlugins, peRootForInlining, sourceLanguagePositionProvider,
                         new ConcurrentHashMap<>(), new ConcurrentHashMap<>(), needsExplicitException, forceLink);
 
         assert !graphBuilderConfig.trackNodeSourcePosition() || graph.trackNodeSourcePosition();
 
-        this.providers = providers;
+        this.graphCacheProviders = graphCacheProviders;
         this.graphBuilderConfig = graphBuilderConfig;
         this.postParsingPhase = postParsingPhase;
         this.persistentGraphCache = persistentGraphCache;
@@ -132,9 +133,9 @@ private EncodedGraph createGraph(ResolvedJavaMethod method, BytecodeProvider int
          * initial graph.
          */
         try (DebugContext.Scope scope = debug.scope("createGraph", graphToEncode)) {
-            new ConvertDeoptimizeToGuardPhase(canonicalizer).apply(graphToEncode, providers);
+            new ConvertDeoptimizeToGuardPhase(canonicalizer).apply(graphToEncode, graphCacheProviders);
             if (GraalOptions.EarlyGVN.getValue(graphToEncode.getOptions())) {
-                new DominatorBasedGlobalValueNumberingPhase(canonicalizer).apply(graphToEncode, providers);
+                new DominatorBasedGlobalValueNumberingPhase(canonicalizer).apply(graphToEncode, graphCacheProviders);
             }
         } catch (Throwable t) {
             throw debug.handle(t);
@@ -166,9 +167,9 @@ private StructuredGraph buildGraph(ResolvedJavaMethod method, BytecodeProvider i
                 throw GraalError.shouldNotReachHere("isn't this dead?"); // ExcludeFromJacocoGeneratedReport
             }
             graphBuilderPhaseInstance.apply(graphToEncode);
-            canonicalizer.apply(graphToEncode, providers);
+            canonicalizer.apply(graphToEncode, graphCacheProviders);
             if (postParsingPhase != null) {
-                postParsingPhase.apply(graphToEncode, providers);
+                postParsingPhase.apply(graphToEncode, graphCacheProviders);
             }
         } catch (Throwable ex) {
             throw debug.handle(ex);

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/PartialEvaluator.java

@@ -114,6 +114,7 @@ public abstract class PartialEvaluator {
     protected boolean persistentEncodedGraphCache;
 
     protected final TruffleConstantFieldProvider constantFieldProvider;
+    private final TruffleCachingConstantFieldProvider graphCacheConstantFieldProvider;
 
     @SuppressWarnings("this-escape")
     public PartialEvaluator(TruffleCompilerConfiguration config, GraphBuilderConfiguration configForRoot) {
@@ -124,6 +125,7 @@ public PartialEvaluator(TruffleCompilerConfiguration config, GraphBuilderConfigu
         this.lastTierDecodingPlugins = createDecodingInvocationPlugins(config.lastTier().partialEvaluator(), configForRoot.getPlugins(), config.lastTier().providers());
         this.nodePlugins = createNodePlugins(configForRoot.getPlugins());
         this.constantFieldProvider = new TruffleConstantFieldProvider(this, getProviders().getConstantFieldProvider());
+        this.graphCacheConstantFieldProvider = new TruffleCachingConstantFieldProvider(this, getProviders().getConstantFieldProvider());
     }
 
     protected void initialize(OptionValues options) {
@@ -401,14 +403,14 @@ protected PEGraphDecoder createGraphDecoder(TruffleTierContext context, Invocati
         DeoptimizeOnExceptionPhase postParsingPhase = new DeoptimizeOnExceptionPhase(
                         method -> getMethodInfo(method).inlineForPartialEvaluation() == InlineKind.DO_NOT_INLINE_WITH_SPECULATIVE_EXCEPTION);
 
-        Providers baseProviders = config.lastTier().providers();
-        Providers compilationUnitProviders = config.lastTier().providers().copyWith(constantFieldProvider);
+        Providers graphCacheProviders = config.lastTier().providers().copyWith(graphCacheConstantFieldProvider);
+        Providers decoderProviders = config.lastTier().providers().copyWith(constantFieldProvider);
 
         assert !allowAssumptionsDuringParsing || !persistentEncodedGraphCache;
-        return new CachingPEGraphDecoder(config.architecture(), context.graph, compilationUnitProviders, newConfig,
+        return new CachingPEGraphDecoder(config.architecture(), context.graph, graphCacheProviders, decoderProviders, newConfig,
                         loopExplosionPlugin, decodingPlugins, inlineInvokePlugins, parameterPlugin, nodePluginList, types.OptimizedCallTarget_callInlined,
                         sourceLanguagePositionProvider, postParsingPhase, graphCache, createCachedGraphScope,
-                        createGraphBuilderPhaseInstance(compilationUnitProviders, newConfig, TruffleCompilerImpl.Optimizations),
+                        createGraphBuilderPhaseInstance(graphCacheProviders, newConfig, TruffleCompilerImpl.Optimizations),
                         allowAssumptionsDuringParsing, false, true);
     }
 

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/TruffleCachingConstantFieldProvider.java

@@ -0,0 +1,85 @@
+/*
+ * Copyright (c) 2013, 2025, 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 jdk.graal.compiler.truffle;
+
+import com.oracle.truffle.compiler.ConstantFieldInfo;
+
+import jdk.graal.compiler.core.common.spi.ConstantFieldProvider;
+import jdk.graal.compiler.nodes.spi.CanonicalizerTool;
+import jdk.graal.compiler.replacements.CachingPEGraphDecoder;
+import jdk.vm.ci.meta.ResolvedJavaField;
+
+/**
+ * Constant field provider used for parsing into the graph cache on HotSpot in
+ * {@link CachingPEGraphDecoder}.
+ */
+final class TruffleCachingConstantFieldProvider implements ConstantFieldProvider {
+
+    private final PartialEvaluator partialEvaluator;
+    private final ConstantFieldProvider delegate;
+
+    TruffleCachingConstantFieldProvider(PartialEvaluator partialEvaluator, ConstantFieldProvider delegate) {
+        this.partialEvaluator = partialEvaluator;
+        this.delegate = delegate;
+    }
+
+    @Override
+    public <T> T readConstantField(ResolvedJavaField field, ConstantFieldTool<T> tool) {
+        boolean isStaticField = field.isStatic();
+        if (!isStaticField && tool.getReceiver().isNull()) {
+            return null;
+        }
+        ConstantFieldInfo info = partialEvaluator.getConstantFieldInfo(field);
+        if (info != null) {
+            /*
+             * Non-null info means the field is annotated by one of the
+             * annotations @CompilationFinal, @Child or @Children. We are not folding such fields
+             * for the code cache on HotSpot. Not even static final fields. We delay this to the
+             * partial evaluation phase to ensure we are not losing the annotation information and
+             * are not reading values that are not yet stable when creating the graph for the cache.
+             * For example, a static final array annotated by '@CompilationFinal(dimensions = 1)':
+             * We cannot fold the array with 'stableDimension=1', because for the cache, the first
+             * dimension is not stable. We cannot fold it with 'stableDimension=0' either, because
+             * we would lose the information about the first dimension being stable for the partial
+             * evaluation phase.
+             **/
+            return null;
+        } else {
+            // otherwise do regular constant folding.
+            return delegate.readConstantField(field, tool);
+        }
+    }
+
+    @Override
+    public boolean maybeFinal(ResolvedJavaField field) {
+        return delegate.maybeFinal(field);
+    }
+
+    @Override
+    public boolean isTrustedFinal(CanonicalizerTool tool, ResolvedJavaField field) {
+        return delegate.isTrustedFinal(tool, field);
+    }
+
+}

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/TruffleConstantFieldProvider.java

@@ -32,6 +32,11 @@
 import jdk.vm.ci.meta.JavaKind;
 import jdk.vm.ci.meta.ResolvedJavaField;
 
+/**
+ * Constant field provider used for Truffle partial evaluation.
+ *
+ * @see TruffleCachingConstantFieldProvider
+ */
 final class TruffleConstantFieldProvider implements ConstantFieldProvider {
 
     private final PartialEvaluator partialEvaluator;