Clean up creation of object initialization memory barriers

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java

@@ -607,7 +607,7 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec
                     // reads don't bypass the ArrayCopyCallNode above.
                     b.pop(JavaKind.Object);
                     b.addPush(JavaKind.Object, new PublishWritesNode(newArray));
-                    b.add(new MembarNode(MembarNode.FenceKind.ALLOCATION_INIT, LocationIdentity.init()));
+                    b.add(MembarNode.forInitialization());
                 }
                 return true;
             }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/MembarNode.java

@@ -33,6 +33,7 @@
 import org.graalvm.word.LocationIdentity;
 
 import jdk.graal.compiler.core.common.type.StampFactory;
+import jdk.graal.compiler.debug.Assertions;
 import jdk.graal.compiler.graph.NodeClass;
 import jdk.graal.compiler.nodeinfo.NodeInfo;
 import jdk.graal.compiler.nodes.FixedWithNextNode;
@@ -109,7 +110,7 @@ public enum FenceKind {
          *         newly allocated object visible to a different thread.
          */
         public boolean isInit() {
-            return this == ALLOCATION_INIT || this == CONSTRUCTOR_FREEZE;
+            return this == ALLOCATION_INIT;
         }
 
         FenceKind(int barriers) {
@@ -127,10 +128,19 @@ public MembarNode(FenceKind fence) {
 
     public MembarNode(FenceKind fence, LocationIdentity location) {
         super(TYPE, StampFactory.forVoid());
+        assert fence.isInit() == location.isInit() : Assertions.errorMessage(fence, location);
         this.fence = fence;
         this.location = location;
     }
 
+    /**
+     * Creates a new {@link MembarNode} to be placed after the initialization of one or more newly
+     * allocated objects.
+     */
+    public static MembarNode forInitialization() {
+        return new MembarNode(FenceKind.ALLOCATION_INIT, LocationIdentity.init());
+    }
+
     public FenceKind getFenceKind() {
         return fence;
     }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/memory/MemoryKill.java

@@ -24,11 +24,12 @@
  */
 package jdk.graal.compiler.nodes.memory;
 
+import org.graalvm.word.LocationIdentity;
+
 import jdk.graal.compiler.graph.MemoryKillMarker;
 import jdk.graal.compiler.graph.Node;
 import jdk.graal.compiler.nodes.FixedNode;
 import jdk.graal.compiler.nodes.ValueNodeInterface;
-import org.graalvm.word.LocationIdentity;
 
 /**
  * This interface marks nodes that kill a set of memory locations represented by
@@ -77,6 +78,11 @@ static boolean isMultiMemoryKill(Node n) {
         return n instanceof MultiMemoryKill;
     }
 
+    /**
+     * @return true iff this node kills {@link LocationIdentity#INIT_LOCATION}.
+     */
+    boolean killsInit();
+
     static SingleMemoryKill asSingleMemoryKill(Node n) {
         assert isSingleMemoryKill(n);
         return (SingleMemoryKill) n;

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/memory/MultiMemoryKill.java

@@ -40,4 +40,13 @@ public interface MultiMemoryKill extends MemoryKill {
      */
     LocationIdentity[] getKilledLocationIdentities();
 
+    @Override
+    default boolean killsInit() {
+        for (LocationIdentity identity : getKilledLocationIdentities()) {
+            if (identity.isInit()) {
+                return true;
+            }
+        }
+        return false;
+    }
 }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/memory/SingleMemoryKill.java

@@ -40,4 +40,8 @@ public interface SingleMemoryKill extends MemoryKill {
      */
     LocationIdentity getKilledLocationIdentity();
 
+    @Override
+    default boolean killsInit() {
+        return getKilledLocationIdentity().isInit();
+    }
 }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/InitMemoryVerificationPhase.java

@@ -46,8 +46,6 @@
 import jdk.graal.compiler.nodes.java.AbstractNewObjectNode;
 import jdk.graal.compiler.nodes.memory.MemoryAnchorNode;
 import jdk.graal.compiler.nodes.memory.MemoryKill;
-import jdk.graal.compiler.nodes.memory.MultiMemoryKill;
-import jdk.graal.compiler.nodes.memory.SingleMemoryKill;
 import jdk.graal.compiler.nodes.spi.CoreProviders;
 import jdk.graal.compiler.phases.RecursivePhase;
 import jdk.graal.compiler.phases.VerifyPhase;
@@ -88,23 +86,6 @@ public Optional<NotApplicable> notApplicableTo(GraphState graphState) {
      * (not protected by a barrier) at any point in the control flow graph..
      */
     private static class InitBarrierVerificationClosure extends ReentrantNodeIterator.NodeIteratorClosure<Integer> {
-        private static boolean killsInitMemory(SingleMemoryKill kill) {
-            return kill.getKilledLocationIdentity().isInit();
-        }
-
-        private static boolean killsInitMemory(MultiMemoryKill kill) {
-            if (kill instanceof MemoryAnchorNode) {
-                // Can ignore as it doesn't actually write
-                return false;
-            }
-            for (LocationIdentity identity : kill.getKilledLocationIdentities()) {
-                if (identity.isInit()) {
-                    return true;
-                }
-            }
-            return false;
-        }
-
         @Override
         protected Integer processNode(FixedNode node, Integer kills) {
             int liveKills = kills;
@@ -114,10 +95,11 @@ protected Integer processNode(FixedNode node, Integer kills) {
                 } else if (liveKills > 0) {
                     throw new VerificationError("%s is a non-init barrier, but there are %d live init writes", memBar, liveKills);
                 }
-            } else if (MemoryKill.isSingleMemoryKill(node) && killsInitMemory(MemoryKill.asSingleMemoryKill(node))) {
-                liveKills++;
-            } else if (MemoryKill.isMultiMemoryKill(node) && killsInitMemory(MemoryKill.asMultiMemoryKill(node))) {
-                liveKills++;
+            } else if (MemoryKill.isMemoryKill(node) && ((MemoryKill) node).killsInit()) {
+                // memory anchors don't actually perform any writes
+                if (!(node instanceof MemoryAnchorNode)) {
+                    liveKills++;
+                }
             } else if (node instanceof ReturnNode && liveKills > 0) {
                 throw new VerificationError("%d writes to init memory not guarded by an init barrier at node %s", liveKills, node);
             }

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

@@ -111,7 +111,6 @@
 import jdk.graal.compiler.nodes.extended.LoadHubNode;
 import jdk.graal.compiler.nodes.extended.LoadHubOrNullNode;
 import jdk.graal.compiler.nodes.extended.MembarNode;
-import jdk.graal.compiler.nodes.extended.MembarNode.FenceKind;
 import jdk.graal.compiler.nodes.extended.ObjectIsArrayNode;
 import jdk.graal.compiler.nodes.extended.PublishWritesNode;
 import jdk.graal.compiler.nodes.extended.RawLoadNode;
@@ -635,7 +634,7 @@ protected void lowerArrayLengthNode(ArrayLengthNode arrayLengthNode, LoweringToo
 
     /**
      * Creates a read node that read the array length and is guarded by a null-check.
-     *
+     * <p>
      * The created node is placed before {@code before} in the CFG.
      */
     private ReadNode createReadArrayLength(ValueNode array, FixedNode before, LoweringTool tool) {
@@ -1170,25 +1169,9 @@ public void finishAllocatedObjects(LoweringTool tool, FixedWithNextNode insertAf
             }
         }
         assert commit.hasNoUsages();
-        insertAllocationBarrier(insertAfter, commit, graph);
-    }
 
-    /**
-     * Insert the required {@link FenceKind#ALLOCATION_INIT} barrier for an allocation.
-     * Alternatively, issue a {@link FenceKind#CONSTRUCTOR_FREEZE} required for final fields if any
-     * final fields are being written.
-     */
-    private static void insertAllocationBarrier(FixedWithNextNode insertAfter, CommitAllocationNode commit, StructuredGraph graph) {
-        FenceKind fence = FenceKind.ALLOCATION_INIT;
-        outer: for (VirtualObjectNode vobj : commit.getVirtualObjects()) {
-            for (ResolvedJavaField field : vobj.type().getInstanceFields(true)) {
-                if (field.isFinal()) {
-                    fence = FenceKind.CONSTRUCTOR_FREEZE;
-                    break outer;
-                }
-            }
-        }
-        graph.addAfterFixed(insertAfter, graph.add(new MembarNode(fence, LocationIdentity.init())));
+        // Insert the required ALLOCATION_INIT barrier after all objects are initialized.
+        graph.addAfterFixed(insertAfter, graph.add(MembarNode.forInitialization()));
     }
 
     public abstract int fieldOffset(ResolvedJavaField field);

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/StoredContinuationAccess.java

@@ -28,6 +28,7 @@
 import org.graalvm.nativeimage.StackValue;
 import org.graalvm.nativeimage.c.function.CodePointer;
 import org.graalvm.nativeimage.c.struct.RawStructure;
+import org.graalvm.word.LocationIdentity;
 import org.graalvm.word.Pointer;
 import org.graalvm.word.PointerBase;
 import org.graalvm.word.UnsignedWord;
@@ -165,7 +166,7 @@ private static void setIP(StoredContinuation cont, CodePointer ip) {
          * the ip is only visible after all the stores that fill in the stack data. To guarantee
          * that, we issue a STORE_STORE memory barrier before setting the ip.
          */
-        MembarNode.memoryBarrier(MembarNode.FenceKind.ALLOCATION_INIT);
+        MembarNode.memoryBarrier(MembarNode.FenceKind.ALLOCATION_INIT, LocationIdentity.INIT_LOCATION);
         cont.ip = ip;
     }