[GR-12587] Fix incorrect marking of Graph as containing position info…

…rmation.

PullRequest: graal/2492

compiler/src/org.graalvm.compiler.graph/src/org/graalvm/compiler/graph/Graph.java

@@ -25,9 +25,6 @@
 package org.graalvm.compiler.graph;
 
 import static org.graalvm.compiler.core.common.GraalOptions.TrackNodeInsertion;
-import static org.graalvm.compiler.graph.Graph.SourcePositionTracking.Default;
-import static org.graalvm.compiler.graph.Graph.SourcePositionTracking.Track;
-import static org.graalvm.compiler.graph.Graph.SourcePositionTracking.UpdateOnly;
 import static org.graalvm.compiler.nodeinfo.NodeCycles.CYCLES_IGNORED;
 import static org.graalvm.compiler.nodeinfo.NodeSize.SIZE_IGNORED;
 
@@ -75,13 +72,6 @@ private enum FreezeState {
         DeepFreeze
     }
 
-    public enum SourcePositionTracking {
-        Default,
-        Ignore,
-        UpdateOnly,
-        Track
-    }
-
     public final String name;
 
     /**
@@ -97,7 +87,7 @@ public enum SourcePositionTracking {
     /**
      * Records if updating of node source information is required when performing inlining.
      */
-    protected SourcePositionTracking trackNodeSourcePosition;
+    protected boolean trackNodeSourcePosition;
 
     /**
      * The number of valid entries in {@link #nodes}.
@@ -224,38 +214,26 @@ public DebugCloseable withoutNodeSourcePosition() {
         return new NodeSourcePositionScope(null);
     }
 
-    /**
-     * Determines if this graph might contain nodes with source information. This is mainly useful
-     * to short circuit logic for updating those positions after inlining since that requires
-     * visiting every node in the graph.
-     */
-    public boolean updateNodeSourcePosition() {
-        return trackNodeSourcePosition == Track || trackNodeSourcePosition == UpdateOnly;
-    }
-
     public boolean trackNodeSourcePosition() {
-        return trackNodeSourcePosition == Track;
+        return trackNodeSourcePosition;
     }
 
     public void setTrackNodeSourcePosition() {
-        if (trackNodeSourcePosition != Track) {
-            assert trackNodeSourcePosition == Default : trackNodeSourcePosition;
-            trackNodeSourcePosition = Track;
+        if (!trackNodeSourcePosition) {
+            assert getNodeCount() == 1 : "can't change the value after nodes have been added";
+            trackNodeSourcePosition = true;
         }
     }
 
-    public static SourcePositionTracking trackNodeSourcePositionDefault(OptionValues options, DebugContext debug) {
-        if (GraalOptions.TrackNodeSourcePosition.getValue(options) || debug.isDumpEnabledForMethod()) {
-            return Track;
-        }
-        return Default;
+    public static boolean trackNodeSourcePositionDefault(OptionValues options, DebugContext debug) {
+        return (GraalOptions.TrackNodeSourcePosition.getValue(options) || debug.isDumpEnabledForMethod());
     }
 
     /**
      * Creates an empty Graph with no name.
      */
     public Graph(OptionValues options, DebugContext debug) {
-        this(null, options, debug);
+        this(null, options, debug, false);
     }
 
     /**
@@ -276,13 +254,13 @@ public static boolean isModificationCountsEnabled() {
      *
      * @param name the name of the graph, used for debugging purposes
      */
-    public Graph(String name, OptionValues options, DebugContext debug) {
+    public Graph(String name, OptionValues options, DebugContext debug, boolean trackNodeSourcePosition) {
         nodes = new Node[INITIAL_NODES_SIZE];
         iterableNodesFirst = new ArrayList<>(NodeClass.allocatedNodeIterabledIds());
         iterableNodesLast = new ArrayList<>(NodeClass.allocatedNodeIterabledIds());
         this.name = name;
         this.options = options;
-        this.trackNodeSourcePosition = trackNodeSourcePositionDefault(options, debug);
+        this.trackNodeSourcePosition = trackNodeSourcePosition || trackNodeSourcePositionDefault(options, debug);
         assert debug != null;
         this.debug = debug;
 
@@ -385,10 +363,7 @@ public final Graph copy(String newName, DebugContext debugForCopy) {
      *            accessed by multiple threads).
      */
     protected Graph copy(String newName, Consumer<UnmodifiableEconomicMap<Node, Node>> duplicationMapCallback, DebugContext debugForCopy) {
-        Graph copy = new Graph(newName, options, debugForCopy);
-        if (trackNodeSourcePosition()) {
-            copy.setTrackNodeSourcePosition();
-        }
+        Graph copy = new Graph(newName, options, debugForCopy, trackNodeSourcePosition());
         UnmodifiableEconomicMap<Node, Node> duplicates = copy.addDuplicates(getNodes(), this, this.getNodeCount(), (EconomicMap<Node, Node>) null);
         if (duplicationMapCallback != null) {
             duplicationMapCallback.accept(duplicates);
@@ -555,7 +530,7 @@ public enum NodeEvent {
         /**
          * A node was removed from the graph.
          */
-        NODE_REMOVED;
+        NODE_REMOVED
     }
 
     /**

compiler/src/org.graalvm.compiler.graph/src/org/graalvm/compiler/graph/Node.java

@@ -1023,7 +1023,7 @@ final Node clone(Graph into, EnumSet<Edges.Type> edgesToCopy) {
         }
         newNode.graph = into;
         newNode.id = INITIAL_ID;
-        if (getNodeSourcePosition() != null && (into == null || into.updateNodeSourcePosition())) {
+        if (getNodeSourcePosition() != null && (into == null || into.trackNodeSourcePosition())) {
             newNode.setNodeSourcePosition(getNodeSourcePosition());
         }
         if (into != null) {

compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotSuitesProvider.java

@@ -30,8 +30,8 @@
 import static org.graalvm.compiler.core.phases.HighTier.Options.Inline;
 
 import java.util.ListIterator;
-import org.graalvm.compiler.debug.Assertions;
 
+import org.graalvm.compiler.debug.Assertions;
 import org.graalvm.compiler.hotspot.GraalHotSpotVMConfig;
 import org.graalvm.compiler.hotspot.HotSpotBackend;
 import org.graalvm.compiler.hotspot.HotSpotGraalRuntimeProvider;
@@ -140,13 +140,10 @@ private boolean appendGraphEncoderTest(PhaseSuite<HighTierContext> suite) {
             protected void run(StructuredGraph graph, HighTierContext context) {
                 EncodedGraph encodedGraph = GraphEncoder.encodeSingleGraph(graph, runtime.getTarget().arch);
 
-                StructuredGraph targetGraph = new StructuredGraph.Builder(graph.getOptions(), graph.getDebug(), AllowAssumptions.YES).method(graph.method()).build();
+                StructuredGraph targetGraph = new StructuredGraph.Builder(graph.getOptions(), graph.getDebug(), AllowAssumptions.YES).method(graph.method()).trackNodeSourcePosition(
+                                graph.trackNodeSourcePosition()).build();
                 SimplifyingGraphDecoder graphDecoder = new SimplifyingGraphDecoder(runtime.getTarget().arch, targetGraph, context.getMetaAccess(), context.getConstantReflection(),
                                 context.getConstantFieldProvider(), context.getStampProvider(), !ImmutableCode.getValue(graph.getOptions()));
-
-                if (graph.trackNodeSourcePosition()) {
-                    targetGraph.setTrackNodeSourcePosition();
-                }
                 graphDecoder.decode(encodedGraph);
             }
 

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/GraphEncoder.java

@@ -440,11 +440,9 @@ public static boolean verifyEncoding(StructuredGraph originalGraph, EncodedGraph
         StructuredGraph decodedGraph = new StructuredGraph.Builder(originalGraph.getOptions(), debug, AllowAssumptions.YES).
                         method(originalGraph.method()).
                         setIsSubstitution(originalGraph.isSubstitution()).
+                        trackNodeSourcePosition(originalGraph.trackNodeSourcePosition()).
                         build();
         // @formatter:off
-        if (originalGraph.trackNodeSourcePosition()) {
-            decodedGraph.setTrackNodeSourcePosition();
-        }
         GraphDecoder decoder = new GraphDecoder(architecture, decodedGraph);
         decoder.decode(encodedGraph);
 

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/StructuredGraph.java

@@ -24,8 +24,6 @@
  */
 package org.graalvm.compiler.nodes;
 
-import static org.graalvm.compiler.graph.Graph.SourcePositionTracking.Default;
-
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Iterator;
@@ -184,7 +182,7 @@ public static class Builder {
         private int entryBCI = JVMCICompiler.INVOCATION_ENTRY_BCI;
         private boolean useProfilingInfo = true;
         private boolean recordInlinedMethods = true;
-        private SourcePositionTracking trackNodeSourcePosition = Default;
+        private boolean trackNodeSourcePosition;
         private final OptionValues options;
         private Cancellable cancellable = null;
         private final DebugContext debug;
@@ -295,14 +293,9 @@ public Builder recordInlinedMethods(boolean flag) {
             return this;
         }
 
-        public Builder trackNodeSourcePosition(SourcePositionTracking tracking) {
-            this.trackNodeSourcePosition = tracking;
-            return this;
-        }
-
         public Builder trackNodeSourcePosition(boolean flag) {
             if (flag) {
-                this.trackNodeSourcePosition = SourcePositionTracking.Track;
+                this.trackNodeSourcePosition = true;
             }
             return this;
         }
@@ -398,13 +391,13 @@ private StructuredGraph(String name,
                     boolean useProfilingInfo,
                     boolean isSubstitution,
                     List<ResolvedJavaMethod> methods,
-                    SourcePositionTracking trackNodeSourcePosition,
+                    boolean trackNodeSourcePosition,
                     CompilationIdentifier compilationId,
                     OptionValues options,
                     DebugContext debug,
                     Cancellable cancellable,
                     NodeSourcePosition context) {
-        super(name, options, debug);
+        super(name, options, debug, trackNodeSourcePosition);
         this.setStart(add(new StartNode()));
         this.rootMethod = method;
         this.graphId = uniqueGraphIds.incrementAndGet();
@@ -416,8 +409,6 @@ private StructuredGraph(String name,
         this.useProfilingInfo = useProfilingInfo;
         this.isSubstitution = isSubstitution;
         assert checkIsSubstitutionInvariants(method, isSubstitution);
-        this.trackNodeSourcePosition = trackNodeSourcePosition;
-        assert trackNodeSourcePosition != null;
         this.cancellable = cancellable;
         this.inliningLog = new InliningLog(rootMethod, GraalOptions.TraceInlining.getValue(options));
         this.callerContext = context;

compiler/src/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/GraphKit.java

@@ -356,10 +356,7 @@ public void inline(InvokeNode invoke, String reason, String phase) {
         Plugins plugins = new Plugins(graphBuilderPlugins);
         GraphBuilderConfiguration config = GraphBuilderConfiguration.getSnippetDefault(plugins);
 
-        StructuredGraph calleeGraph = new StructuredGraph.Builder(invoke.getOptions(), invoke.getDebug()).method(method).build();
-        if (invoke.graph().trackNodeSourcePosition()) {
-            calleeGraph.setTrackNodeSourcePosition();
-        }
+        StructuredGraph calleeGraph = new StructuredGraph.Builder(invoke.getOptions(), invoke.getDebug()).method(method).trackNodeSourcePosition(invoke.graph().trackNodeSourcePosition()).build();
         IntrinsicContext initialReplacementContext = new IntrinsicContext(method, method, providers.getReplacements().getDefaultReplacementBytecodeProvider(), INLINE_AFTER_PARSING);
         GraphBuilderPhase.Instance instance = createGraphBuilderInstance(metaAccess, providers.getStampProvider(), providers.getConstantReflection(), providers.getConstantFieldProvider(), config,
                         OptimisticOptimizations.NONE,

compiler/src/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/IntrinsicGraphBuilder.java

@@ -31,9 +31,9 @@
 import org.graalvm.compiler.core.common.type.StampFactory;
 import org.graalvm.compiler.core.common.type.StampPair;
 import org.graalvm.compiler.core.common.type.TypeReference;
+import org.graalvm.compiler.debug.DebugCloseable;
 import org.graalvm.compiler.debug.DebugContext;
 import org.graalvm.compiler.debug.GraalError;
-import org.graalvm.compiler.debug.DebugCloseable;
 import org.graalvm.compiler.graph.NodeSourcePosition;
 import org.graalvm.compiler.nodes.CallTargetNode;
 import org.graalvm.compiler.nodes.CallTargetNode.InvokeKind;
@@ -94,8 +94,7 @@ protected IntrinsicGraphBuilder(OptionValues options, DebugContext debug, MetaAc
         this.stampProvider = stampProvider;
         this.code = code;
         this.method = code.getMethod();
-        this.graph = new StructuredGraph.Builder(options, debug, allowAssumptions).method(method).setIsSubstitution(true).build();
-        this.graph.setTrackNodeSourcePosition();
+        this.graph = new StructuredGraph.Builder(options, debug, allowAssumptions).method(method).setIsSubstitution(true).trackNodeSourcePosition(true).build();
         this.invokeBci = invokeBci;
         this.lastInstr = graph.start();
 

compiler/src/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/SnippetTemplate.java

@@ -49,7 +49,6 @@
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Predicate;
 
-import jdk.vm.ci.meta.ConstantReflectionProvider;
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.collections.EconomicSet;
 import org.graalvm.collections.Equivalence;
@@ -142,6 +141,7 @@
 
 import jdk.vm.ci.code.TargetDescription;
 import jdk.vm.ci.meta.Constant;
+import jdk.vm.ci.meta.ConstantReflectionProvider;
 import jdk.vm.ci.meta.JavaConstant;
 import jdk.vm.ci.meta.JavaKind;
 import jdk.vm.ci.meta.Local;
@@ -714,7 +714,8 @@ protected SnippetTemplate(OptionValues options, DebugContext debug, final Provid
         this.info = args.info;
 
         Object[] constantArgs = getConstantArgs(args);
-        StructuredGraph snippetGraph = providers.getReplacements().getSnippet(args.info.method, args.info.original, constantArgs, trackNodeSourcePosition, replacee.getNodeSourcePosition());
+        boolean shouldTrackNodeSourcePosition1 = trackNodeSourcePosition || (providers.getCodeCache() != null && providers.getCodeCache().shouldDebugNonSafepoints());
+        StructuredGraph snippetGraph = providers.getReplacements().getSnippet(args.info.method, args.info.original, constantArgs, shouldTrackNodeSourcePosition1, replacee.getNodeSourcePosition());
 
         ResolvedJavaMethod method = snippetGraph.method();
         Signature signature = method.getSignature();
@@ -725,9 +726,6 @@ protected SnippetTemplate(OptionValues options, DebugContext debug, final Provid
         final StructuredGraph snippetCopy = new StructuredGraph.Builder(options, debug).name(snippetGraph.name).method(snippetGraph.method()).trackNodeSourcePosition(
                         snippetGraph.trackNodeSourcePosition()).setIsSubstitution(true).build();
         assert !GraalOptions.TrackNodeSourcePosition.getValue(options) || snippetCopy.trackNodeSourcePosition();
-        if (providers.getCodeCache() != null && providers.getCodeCache().shouldDebugNonSafepoints()) {
-            snippetCopy.setTrackNodeSourcePosition();
-        }
         try (DebugContext.Scope scope = debug.scope("SpecializeSnippet", snippetCopy)) {
             if (!snippetGraph.isUnsafeAccessTrackingEnabled()) {
                 snippetCopy.disableUnsafeAccessTracking();

compiler/src/org.graalvm.compiler.truffle.compiler/src/org/graalvm/compiler/truffle/compiler/PartialEvaluator.java

@@ -24,18 +24,18 @@
  */
 package org.graalvm.compiler.truffle.compiler;
 
-import java.net.URI;
 import static org.graalvm.compiler.nodes.graphbuilderconf.InlineInvokePlugin.InlineInfo.createStandardInlineInfo;
-import static org.graalvm.compiler.truffle.compiler.TruffleCompilerOptions.PrintTruffleExpansionHistogram;
-import static org.graalvm.compiler.truffle.compiler.TruffleCompilerOptions.TraceTrufflePerformanceWarnings;
 import static org.graalvm.compiler.truffle.compiler.SharedTruffleCompilerOptions.TraceTruffleStackTraceLimit;
 import static org.graalvm.compiler.truffle.compiler.SharedTruffleCompilerOptions.TruffleFunctionInlining;
+import static org.graalvm.compiler.truffle.compiler.SharedTruffleCompilerOptions.TrufflePerformanceWarningsAreFatal;
+import static org.graalvm.compiler.truffle.compiler.TruffleCompilerOptions.PrintTruffleExpansionHistogram;
+import static org.graalvm.compiler.truffle.compiler.TruffleCompilerOptions.TraceTrufflePerformanceWarnings;
 import static org.graalvm.compiler.truffle.compiler.TruffleCompilerOptions.TruffleInlineAcrossTruffleBoundary;
 import static org.graalvm.compiler.truffle.compiler.TruffleCompilerOptions.TruffleInstrumentBoundaries;
 import static org.graalvm.compiler.truffle.compiler.TruffleCompilerOptions.TruffleInstrumentBranches;
 import static org.graalvm.compiler.truffle.compiler.TruffleCompilerOptions.TruffleIterativePartialEscape;
-import static org.graalvm.compiler.truffle.compiler.SharedTruffleCompilerOptions.TrufflePerformanceWarningsAreFatal;
 
+import java.net.URI;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -97,6 +97,7 @@
 import org.graalvm.compiler.truffle.common.CompilableTruffleAST;
 import org.graalvm.compiler.truffle.common.TruffleCompilerRuntime;
 import org.graalvm.compiler.truffle.common.TruffleInliningPlan;
+import org.graalvm.compiler.truffle.common.TruffleSourceLanguagePosition;
 import org.graalvm.compiler.truffle.compiler.debug.HistogramInlineInvokePlugin;
 import org.graalvm.compiler.truffle.compiler.nodes.TruffleAssumption;
 import org.graalvm.compiler.truffle.compiler.nodes.asserts.NeverPartOfCompilationNode;
@@ -118,7 +119,6 @@
 import jdk.vm.ci.meta.ResolvedJavaMethod;
 import jdk.vm.ci.meta.ResolvedJavaType;
 import jdk.vm.ci.meta.SpeculationLog;
-import org.graalvm.compiler.truffle.common.TruffleSourceLanguagePosition;
 
 /**
  * Class performing the partial evaluation starting from the root node of an AST.
@@ -230,6 +230,7 @@ public StructuredGraph createGraph(DebugContext debug, final CompilableTruffleAS
                         method(rootMethod).
                         speculationLog(log).
                         compilationId(compilationId).
+                        trackNodeSourcePosition(configForParsing.trackNodeSourcePosition()).
                         cancellable(cancellable);
         // @formatter:on
         builder = customizeStructuredGraphBuilder(builder);
@@ -474,9 +475,6 @@ protected void doGraphPE(CompilableTruffleAST compilable, StructuredGraph graph,
             inlineInvokePlugins = new InlineInvokePlugin[]{replacements, inlineInvokePlugin};
         }
 
-        if (configForParsing.trackNodeSourcePosition()) {
-            graph.setTrackNodeSourcePosition();
-        }
         SourceLanguagePositionProvider sourceLanguagePosition = new TruffleSourceLanguagePositionProvider(inliningDecision);
         PEGraphDecoder decoder = createGraphDecoder(graph, tierContext, loopExplosionPlugin, decodingInvocationPlugins, inlineInvokePlugins, parameterPlugin, nodePlugins, callInlinedMethod,
                         sourceLanguagePosition);