[GR-32774] Ensure source position for new instance node is tracked.

Munish Chouhan · Munish Chouhan · commit 68fa094f3b5e · 2021-08-12T11:29:03.000Z
PullRequest: graal/9487
diff --git a/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/extended/BoxNode.java b/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/extended/BoxNode.java
@@ -136,7 +136,6 @@ public ValueNode canonical(CanonicalizerTool tool, ValueNode forValue) {
 
     protected VirtualBoxingNode createVirtualBoxingNode() {
         VirtualBoxingNode node = new VirtualBoxingNode(StampTool.typeOrNull(stamp(NodeView.DEFAULT)), boxingKind);
-        node.setNodeSourcePosition(getNodeSourcePosition());
         return node;
     }
 
@@ -151,7 +150,7 @@ public void virtualize(VirtualizerTool tool) {
         VirtualBoxingNode newVirtual = createVirtualBoxingNode();
         assert newVirtual.getFields().length == 1;
 
-        tool.createVirtualObject(newVirtual, new ValueNode[]{alias}, Collections.<MonitorIdNode> emptyList(), false);
+        tool.createVirtualObject(newVirtual, new ValueNode[]{alias}, Collections.<MonitorIdNode> emptyList(), getNodeSourcePosition(), false);
         tool.replaceWithVirtual(newVirtual);
     }
 
diff --git a/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/java/NewArrayNode.java b/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/java/NewArrayNode.java
@@ -108,7 +108,7 @@ public void virtualize(VirtualizerTool tool) {
                 }
 
                 VirtualObjectNode virtualObject = new VirtualArrayNode(elementType(), constantLength);
-                tool.createVirtualObject(virtualObject, state, Collections.<MonitorIdNode> emptyList(), false);
+                tool.createVirtualObject(virtualObject, state, Collections.<MonitorIdNode> emptyList(), getNodeSourcePosition(), false);
                 tool.replaceWithVirtual(virtualObject);
             }
         }
diff --git a/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/java/NewInstanceNode.java b/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/java/NewInstanceNode.java
@@ -86,7 +86,7 @@ public void virtualize(VirtualizerTool tool) {
             for (int i = 0; i < state.length; i++) {
                 state[i] = ConstantNode.defaultForKind(tool.getMetaAccessExtensionProvider().getStorageKind(fields[i].getType()), graph());
             }
-            tool.createVirtualObject(virtualObject, state, Collections.<MonitorIdNode> emptyList(), false);
+            tool.createVirtualObject(virtualObject, state, Collections.<MonitorIdNode> emptyList(), getNodeSourcePosition(), false);
             tool.replaceWithVirtual(virtualObject);
         }
     }
diff --git a/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/spi/VirtualizerTool.java b/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/spi/VirtualizerTool.java
@@ -29,6 +29,7 @@
 import org.graalvm.compiler.debug.DebugContext;
 import org.graalvm.compiler.debug.GraalError;
 import org.graalvm.compiler.graph.Node;
+import org.graalvm.compiler.graph.NodeSourcePosition;
 import org.graalvm.compiler.nodes.ValueNode;
 import org.graalvm.compiler.nodes.WithExceptionNode;
 import org.graalvm.compiler.nodes.java.MonitorIdNode;
@@ -61,9 +62,10 @@ public interface VirtualizerTool extends CoreProviders {
      * @param virtualObject the new virtual object.
      * @param entryState the initial state of the virtual object's fields.
      * @param locks the initial locking depths.
+     * @param sourcePosition a source position for the new node or null if none is available
      * @param ensureVirtualized true if this object needs to stay virtual
      */
-    void createVirtualObject(VirtualObjectNode virtualObject, ValueNode[] entryState, List<MonitorIdNode> locks, boolean ensureVirtualized);
+    void createVirtualObject(VirtualObjectNode virtualObject, ValueNode[] entryState, List<MonitorIdNode> locks, NodeSourcePosition sourcePosition, boolean ensureVirtualized);
 
     /**
      * Returns a VirtualObjectNode if the given value is aliased with a virtual object that is still
diff --git a/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/util/GraphUtil.java b/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/util/GraphUtil.java
@@ -1159,7 +1159,7 @@ public static void virtualizeArrayCopy(VirtualizerTool tool, ValueNode source, V
         }
         /* Perform the replacement. */
         VirtualArrayNode newVirtualArray = virtualArrayProvider.apply(newComponentType, newLengthInt);
-        tool.createVirtualObject(newVirtualArray, newEntryState, Collections.<MonitorIdNode> emptyList(), false);
+        tool.createVirtualObject(newVirtualArray, newEntryState, Collections.<MonitorIdNode> emptyList(), source.getNodeSourcePosition(), false);
         tool.replaceWithVirtual(newVirtualArray);
     }
 
diff --git a/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/virtual/CommitAllocationNode.java b/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/virtual/CommitAllocationNode.java
@@ -141,7 +141,11 @@ public void virtualize(VirtualizerTool tool) {
         for (int i = 0; i < virtualObjects.size(); i++) {
             VirtualObjectNode virtualObject = virtualObjects.get(i);
             int entryCount = virtualObject.entryCount();
-            tool.createVirtualObject(virtualObject, values.subList(pos, pos + entryCount).toArray(new ValueNode[entryCount]), getLocks(i), ensureVirtual.get(i));
+            /*
+             * n.b. the node source position of virtualObject will have been set when it was
+             * created.
+             */
+            tool.createVirtualObject(virtualObject, values.subList(pos, pos + entryCount).toArray(new ValueNode[entryCount]), getLocks(i), virtualObject.getNodeSourcePosition(), ensureVirtual.get(i));
             pos += entryCount;
         }
         tool.delete();
diff --git a/compiler/src/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/nodes/ObjectClone.java b/compiler/src/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/nodes/ObjectClone.java
@@ -29,6 +29,7 @@
 import org.graalvm.compiler.core.common.type.ObjectStamp;
 import org.graalvm.compiler.core.common.type.Stamp;
 import org.graalvm.compiler.core.common.type.StampFactory;
+import org.graalvm.compiler.graph.NodeSourcePosition;
 import org.graalvm.compiler.nodes.ConstantNode;
 import org.graalvm.compiler.nodes.NodeView;
 import org.graalvm.compiler.nodes.StateSplit;
@@ -95,7 +96,9 @@ default LoadIndexedNode genLoadIndexedNode(Assumptions assumptions, ValueNode or
 
     @Override
     default void virtualize(VirtualizerTool tool) {
-        ValueNode originalAlias = tool.getAlias(getObject());
+        ValueNode original = getObject();
+        ValueNode originalAlias = tool.getAlias(original);
+        NodeSourcePosition sourcePosition = original.getNodeSourcePosition();
         if (originalAlias instanceof VirtualObjectNode) {
             VirtualObjectNode originalVirtual = (VirtualObjectNode) originalAlias;
             if (originalVirtual.type().isCloneableWithAllocation()) {
@@ -104,7 +107,8 @@ default void virtualize(VirtualizerTool tool) {
                     newEntryState[i] = tool.getEntry(originalVirtual, i);
                 }
                 VirtualObjectNode newVirtual = originalVirtual.duplicate();
-                tool.createVirtualObject(newVirtual, newEntryState, Collections.<MonitorIdNode> emptyList(), false);
+                /* n.b. duplicate will replicate the source position so pass null */
+                tool.createVirtualObject(newVirtual, newEntryState, Collections.<MonitorIdNode> emptyList(), null, false);
                 tool.replaceWithVirtual(newVirtual);
             }
         } else {
@@ -122,7 +126,7 @@ default void virtualize(VirtualizerTool tool) {
                     state[i] = load;
                     tool.addNode(load);
                 }
-                tool.createVirtualObject(newVirtual, state, Collections.<MonitorIdNode> emptyList(), false);
+                tool.createVirtualObject(newVirtual, state, Collections.<MonitorIdNode> emptyList(), sourcePosition, false);
                 tool.replaceWithVirtual(newVirtual);
             } else {
                 ValueNode length = findLength(FindLengthMode.SEARCH_ONLY, tool.getConstantReflection());
@@ -145,7 +149,7 @@ default void virtualize(VirtualizerTool tool) {
                         tool.addNode(load);
                     }
                     VirtualObjectNode virtualObject = new VirtualArrayNode(componentType, constantLength);
-                    tool.createVirtualObject(virtualObject, state, Collections.<MonitorIdNode> emptyList(), false);
+                    tool.createVirtualObject(virtualObject, state, Collections.<MonitorIdNode> emptyList(), sourcePosition, false);
                     tool.replaceWithVirtual(virtualObject);
                 }
             }
diff --git a/compiler/src/org.graalvm.compiler.truffle.compiler/src/org/graalvm/compiler/truffle/compiler/nodes/frame/NewFrameNode.java b/compiler/src/org.graalvm.compiler.truffle.compiler/src/org/graalvm/compiler/truffle/compiler/nodes/frame/NewFrameNode.java
@@ -37,6 +37,7 @@
 import org.graalvm.compiler.graph.Node;
 import org.graalvm.compiler.graph.NodeClass;
 import org.graalvm.compiler.graph.NodeInputList;
+import org.graalvm.compiler.graph.NodeSourcePosition;
 import org.graalvm.compiler.nodes.spi.Canonicalizable;
 import org.graalvm.compiler.nodes.spi.CanonicalizerTool;
 import org.graalvm.compiler.nodeinfo.NodeInfo;
@@ -268,12 +269,13 @@ public void virtualize(VirtualizerTool tool) {
             }
         }
 
-        tool.createVirtualObject(virtualFrameObjectArray, objectArrayEntryState, Collections.<MonitorIdNode> emptyList(), false);
+        NodeSourcePosition sourcePosition = getNodeSourcePosition();
+        tool.createVirtualObject(virtualFrameObjectArray, objectArrayEntryState, Collections.<MonitorIdNode> emptyList(), sourcePosition, false);
         if (virtualFramePrimitiveArray != null) {
-            tool.createVirtualObject(virtualFramePrimitiveArray, primitiveArrayEntryState, Collections.<MonitorIdNode> emptyList(), false);
+            tool.createVirtualObject(virtualFramePrimitiveArray, primitiveArrayEntryState, Collections.<MonitorIdNode> emptyList(), sourcePosition, false);
         }
         if (virtualFrameTagArray != null) {
-            tool.createVirtualObject(virtualFrameTagArray, tagArrayEntryState, Collections.<MonitorIdNode> emptyList(), false);
+            tool.createVirtualObject(virtualFrameTagArray, tagArrayEntryState, Collections.<MonitorIdNode> emptyList(), sourcePosition, false);
         }
 
         assert frameFields.length == 5 || frameFields.length == 3;
@@ -293,7 +295,7 @@ public void virtualize(VirtualizerTool tool) {
          * materialized. This can only be lifted by a AllowMaterializeNode, which corresponds to a
          * frame.materialize() call.
          */
-        tool.createVirtualObject(virtualFrame, frameEntryState, Collections.<MonitorIdNode> emptyList(), true);
+        tool.createVirtualObject(virtualFrame, frameEntryState, Collections.<MonitorIdNode> emptyList(), sourcePosition, true);
         tool.replaceWithVirtual(virtualFrame);
     }
 
diff --git a/compiler/src/org.graalvm.compiler.virtual/src/org/graalvm/compiler/virtual/phases/ea/VirtualizerToolImpl.java b/compiler/src/org.graalvm.compiler.virtual/src/org/graalvm/compiler/virtual/phases/ea/VirtualizerToolImpl.java
@@ -30,6 +30,7 @@
 
 import org.graalvm.compiler.debug.DebugContext;
 import org.graalvm.compiler.graph.Node;
+import org.graalvm.compiler.graph.NodeSourcePosition;
 import org.graalvm.compiler.nodes.spi.CanonicalizerTool;
 import org.graalvm.compiler.nodes.ConstantNode;
 import org.graalvm.compiler.nodes.FixedNode;
@@ -290,7 +291,7 @@ public void addNode(ValueNode node) {
     }
 
     @Override
-    public void createVirtualObject(VirtualObjectNode virtualObject, ValueNode[] entryState, List<MonitorIdNode> locks, boolean ensureVirtualized) {
+    public void createVirtualObject(VirtualObjectNode virtualObject, ValueNode[] entryState, List<MonitorIdNode> locks, NodeSourcePosition sourcePosition, boolean ensureVirtualized) {
         VirtualUtil.trace(options, debug, "{{%s}} ", current);
         if (!virtualObject.isAlive()) {
             effects.addFloatingNode(virtualObject, "newVirtualObject");
@@ -309,6 +310,10 @@ public void createVirtualObject(VirtualObjectNode virtualObject, ValueNode[] ent
         closure.addVirtualAlias(virtualObject, virtualObject);
         PartialEscapeClosure.COUNTER_ALLOCATION_REMOVED.increment(debug);
         effects.addVirtualizationDelta(1);
+        if (sourcePosition != null) {
+            assert virtualObject.getNodeSourcePosition() == null || virtualObject.getNodeSourcePosition() == sourcePosition : "unexpected source pos!";
+            virtualObject.setNodeSourcePosition(sourcePosition);
+        }
     }
 
     @Override

Original file line number	Diff line number	Diff line change
`@@ -136,7 +136,6 @@ public ValueNode canonical(CanonicalizerTool tool, ValueNode forValue) {`
`136`	`136`
`137`	`137`	`protected VirtualBoxingNode createVirtualBoxingNode() {`
`138`	`138`	`VirtualBoxingNode node = new VirtualBoxingNode(StampTool.typeOrNull(stamp(NodeView.DEFAULT)), boxingKind);`
`139`		`- node.setNodeSourcePosition(getNodeSourcePosition());`
`140`	`139`	`return node;`
`141`	`140`	`}`
`142`	`141`
`@@ -151,7 +150,7 @@ public void virtualize(VirtualizerTool tool) {`
`151`	`150`	`VirtualBoxingNode newVirtual = createVirtualBoxingNode();`
`152`	`151`	`assert newVirtual.getFields().length == 1;`
`153`	`152`
`154`		`- tool.createVirtualObject(newVirtual, new ValueNode[]{alias}, Collections.<MonitorIdNode> emptyList(), false);`
	`153`	`+ tool.createVirtualObject(newVirtual, new ValueNode[]{alias}, Collections.<MonitorIdNode> emptyList(), getNodeSourcePosition(), false);`
`155`	`154`	`tool.replaceWithVirtual(newVirtual);`
`156`	`155`	`}`
`157`	`156`
Original file line number	Diff line number	Diff line change
`@@ -108,7 +108,7 @@ public void virtualize(VirtualizerTool tool) {`
`108`	`108`	`}`
`109`	`109`
`110`	`110`	`VirtualObjectNode virtualObject = new VirtualArrayNode(elementType(), constantLength);`
`111`		`- tool.createVirtualObject(virtualObject, state, Collections.<MonitorIdNode> emptyList(), false);`
	`111`	`+ tool.createVirtualObject(virtualObject, state, Collections.<MonitorIdNode> emptyList(), getNodeSourcePosition(), false);`
`112`	`112`	`tool.replaceWithVirtual(virtualObject);`
`113`	`113`	`}`
`114`	`114`	`}`
Original file line number	Diff line number	Diff line change
`@@ -86,7 +86,7 @@ public void virtualize(VirtualizerTool tool) {`
`86`	`86`	`for (int i = 0; i < state.length; i++) {`
`87`	`87`	`state[i] = ConstantNode.defaultForKind(tool.getMetaAccessExtensionProvider().getStorageKind(fields[i].getType()), graph());`
`88`	`88`	`}`
`89`		`- tool.createVirtualObject(virtualObject, state, Collections.<MonitorIdNode> emptyList(), false);`
	`89`	`+ tool.createVirtualObject(virtualObject, state, Collections.<MonitorIdNode> emptyList(), getNodeSourcePosition(), false);`
`90`	`90`	`tool.replaceWithVirtual(virtualObject);`
`91`	`91`	`}`
`92`	`92`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1159,7 +1159,7 @@ public static void virtualizeArrayCopy(VirtualizerTool tool, ValueNode source, V`
`1159`	`1159`	`}`
`1160`	`1160`	`/* Perform the replacement. */`
`1161`	`1161`	`VirtualArrayNode newVirtualArray = virtualArrayProvider.apply(newComponentType, newLengthInt);`
`1162`		`- tool.createVirtualObject(newVirtualArray, newEntryState, Collections.<MonitorIdNode> emptyList(), false);`
	`1162`	`+ tool.createVirtualObject(newVirtualArray, newEntryState, Collections.<MonitorIdNode> emptyList(), source.getNodeSourcePosition(), false);`
`1163`	`1163`	`tool.replaceWithVirtual(newVirtualArray);`
`1164`	`1164`	`}`
`1165`	`1165`
Original file line number	Diff line number	Diff line change
`@@ -37,6 +37,7 @@`
`37`	`37`	`import org.graalvm.compiler.graph.Node;`
`38`	`38`	`import org.graalvm.compiler.graph.NodeClass;`
`39`	`39`	`import org.graalvm.compiler.graph.NodeInputList;`
	`40`	`+import org.graalvm.compiler.graph.NodeSourcePosition;`
`40`	`41`	`import org.graalvm.compiler.nodes.spi.Canonicalizable;`
`41`	`42`	`import org.graalvm.compiler.nodes.spi.CanonicalizerTool;`
`42`	`43`	`import org.graalvm.compiler.nodeinfo.NodeInfo;`
`@@ -268,12 +269,13 @@ public void virtualize(VirtualizerTool tool) {`
`268`	`269`	`}`
`269`	`270`	`}`
`270`	`271`
`271`		`- tool.createVirtualObject(virtualFrameObjectArray, objectArrayEntryState, Collections.<MonitorIdNode> emptyList(), false);`
	`272`	`+ NodeSourcePosition sourcePosition = getNodeSourcePosition();`
	`273`	`+ tool.createVirtualObject(virtualFrameObjectArray, objectArrayEntryState, Collections.<MonitorIdNode> emptyList(), sourcePosition, false);`
`272`	`274`	`if (virtualFramePrimitiveArray != null) {`
`273`		`- tool.createVirtualObject(virtualFramePrimitiveArray, primitiveArrayEntryState, Collections.<MonitorIdNode> emptyList(), false);`
	`275`	`+ tool.createVirtualObject(virtualFramePrimitiveArray, primitiveArrayEntryState, Collections.<MonitorIdNode> emptyList(), sourcePosition, false);`
`274`	`276`	`}`
`275`	`277`	`if (virtualFrameTagArray != null) {`
`276`		`- tool.createVirtualObject(virtualFrameTagArray, tagArrayEntryState, Collections.<MonitorIdNode> emptyList(), false);`
	`278`	`+ tool.createVirtualObject(virtualFrameTagArray, tagArrayEntryState, Collections.<MonitorIdNode> emptyList(), sourcePosition, false);`
`277`	`279`	`}`
`278`	`280`
`279`	`281`	`assert frameFields.length == 5 \|\| frameFields.length == 3;`
`@@ -293,7 +295,7 @@ public void virtualize(VirtualizerTool tool) {`
`293`	`295`	`* materialized. This can only be lifted by a AllowMaterializeNode, which corresponds to a`
`294`	`296`	`* frame.materialize() call.`
`295`	`297`	`*/`
`296`		`- tool.createVirtualObject(virtualFrame, frameEntryState, Collections.<MonitorIdNode> emptyList(), true);`
	`298`	`+ tool.createVirtualObject(virtualFrame, frameEntryState, Collections.<MonitorIdNode> emptyList(), sourcePosition, true);`
`297`	`299`	`tool.replaceWithVirtual(virtualFrame);`
`298`	`300`	`}`
`299`	`301`