changeset 10801:4bfbd4be6e7a

Replace custom graph building with snippet for unsafe load lowering
author Christos Kotselidis <christos.kotselidis@oracle.com>
date Wed, 17 Jul 2013 20:23:36 +0200
parents 69e305a5cf09
children e6bb7edcc66c
files graal/com.oracle.graal.hotspot.test/src/com/oracle/graal/hotspot/test/WriteBarrierAdditionTest.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotRuntime.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/UnsafeLoadSnippets.java
diffstat 3 files changed, 109 insertions(+), 92 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.hotspot.test/src/com/oracle/graal/hotspot/test/WriteBarrierAdditionTest.java	Wed Jul 17 19:52:20 2013 +0200
+++ b/graal/com.oracle.graal.hotspot.test/src/com/oracle/graal/hotspot/test/WriteBarrierAdditionTest.java	Wed Jul 17 20:23:36 2013 +0200
@@ -160,7 +160,7 @@
     }
 
     /**
-     * The following test concern the runtime checks of the unsafe loads. In this test, we unsafely
+     * The following test concerns the runtime checks of the unsafe loads. In this test, we unsafely
      * load the java.lang.ref.Reference.referent field so the pre barier has to be executed.
      */
     @Test
@@ -169,7 +169,7 @@
     }
 
     /**
-     * The following test concern the runtime checks of the unsafe loads. In this test, we unsafely
+     * The following test concerns the runtime checks of the unsafe loads. In this test, we unsafely
      * load a matching offset of a wrong object so the pre barier must not be executed.
      */
     @Test
@@ -178,7 +178,7 @@
     }
 
     /**
-     * The following test concern the runtime checks of the unsafe loads. In this test, we unsafely
+     * The following test concerns the runtime checks of the unsafe loads. In this test, we unsafely
      * load a non-matching offset field of the java.lang.ref.Reference object so the pre barier must
      * not be executed.
      */
@@ -187,9 +187,30 @@
         test2("test6Snippet", wr, new Long(32), null);
     }
 
-    @SuppressWarnings("unused")
+    /**
+     * The following test concerns the runtime checks of the unsafe loads. In this test, we unsafely
+     * load a matching offset+disp field of the java.lang.ref.Reference object so the pre barier
+     * must be executed.
+     */
+    @Test
+    public void test10() throws Exception {
+        test2("test6Snippet", wr, new Long(8), new Integer(8));
+    }
+
+    /**
+     * The following test concerns the runtime checks of the unsafe loads. In this test, we unsafely
+     * load a non-matching offset+disp field of the java.lang.ref.Reference object so the pre barier
+     * must not be executed.
+     */
+    @Test
+    public void test9() throws Exception {
+        test2("test6Snippet", wr, new Long(16), new Integer(16));
+    }
+
     public static Object test6Snippet(Object a, Object b, Object c) throws Exception {
-        return UnsafeLoadNode.load(a, 0, ((Long) b).longValue(), Kind.Object);
+        final int offset = (c == null ? 0 : ((Integer) c).intValue());
+        final long displacement = (b == null ? 0 : ((Long) b).longValue());
+        return UnsafeLoadNode.load(a, offset, displacement, Kind.Object);
     }
 
     private HotSpotInstalledCode getInstalledCode(String name) throws Exception {
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotRuntime.java	Wed Jul 17 19:52:20 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotRuntime.java	Wed Jul 17 20:23:36 2013 +0200
@@ -115,6 +115,7 @@
     private WriteBarrierSnippets.Templates writeBarrierSnippets;
     private BoxingSnippets.Templates boxingSnippets;
     private LoadExceptionObjectSnippets.Templates exceptionObjectSnippets;
+    private UnsafeLoadSnippets.Templates unsafeLoadSnippets;
 
     private final Map<ForeignCallDescriptor, HotSpotForeignCallLinkage> foreignCalls = new HashMap<>();
 
@@ -330,6 +331,7 @@
         writeBarrierSnippets = new WriteBarrierSnippets.Templates(this, r, graalRuntime.getTarget());
         boxingSnippets = new BoxingSnippets.Templates(this, r, graalRuntime.getTarget());
         exceptionObjectSnippets = new LoadExceptionObjectSnippets.Templates(this, r, graalRuntime.getTarget());
+        unsafeLoadSnippets = new UnsafeLoadSnippets.Templates(this, r, graalRuntime.getTarget());
 
         r.registerSnippetTemplateCache(new UnsafeArrayCopySnippets.Templates(this, r, graalRuntime.getTarget()));
     }
@@ -626,7 +628,17 @@
         } else if (n instanceof UnsafeLoadNode) {
             UnsafeLoadNode load = (UnsafeLoadNode) n;
             assert load.kind() != Kind.Illegal;
-            lowerUnsafeLoad(load, tool);
+            boolean compress = (!load.object().isNullConstant() && load.accessKind() == Kind.Object);
+            if (addReadBarrier(load, tool)) {
+                unsafeLoadSnippets.lower(load, tool);
+            } else {
+                IndexedLocationNode location = IndexedLocationNode.create(ANY_LOCATION, load.accessKind(), load.displacement(), load.offset(), graph, 1);
+                ReadNode memoryRead = graph.add(new ReadNode(load.object(), location, load.stamp(), WriteBarrierType.NONE, compress));
+                // An unsafe read must not float outside its block otherwise
+                // it may float above an explicit null check on its object.
+                memoryRead.setGuard(AbstractBeginNode.prevBegin(load));
+                graph.replaceFixedWithFixed(load, memoryRead);
+            }
         } else if (n instanceof UnsafeStoreNode) {
             UnsafeStoreNode store = (UnsafeStoreNode) n;
             IndexedLocationNode location = IndexedLocationNode.create(ANY_LOCATION, store.accessKind(), store.displacement(), store.offset(), graph, 1);
@@ -823,6 +835,11 @@
         }
     }
 
+    private static boolean addReadBarrier(UnsafeLoadNode load, LoweringTool tool) {
+        return useG1GC() && load.object().kind() == Kind.Object && load.accessKind() == Kind.Object && !load.object().objectStamp().alwaysNull() && load.object().objectStamp().type() != null &&
+                        !(load.object().objectStamp().type().isArray()) && tool.getLoweringType() == LoweringType.BEFORE_GUARDS;
+    }
+
     private static ReadNode createReadVirtualMethod(StructuredGraph graph, Kind wordKind, ValueNode hub, ResolvedJavaMethod method) {
         HotSpotResolvedJavaMethod hsMethod = (HotSpotResolvedJavaMethod) method;
         assert !hsMethod.getDeclaringClass().isInterface();
@@ -849,92 +866,6 @@
         return graph.add(new WriteNode(object, value, location, WriteBarrierType.NONE, config.useCompressedKlassPointers));
     }
 
-    /**
-     * The following method lowers the unsafe load node. If any GC besides G1 is used, the unsafe
-     * load is lowered normally to a read node. However, if the G1 is used and the unsafe load could
-     * not be canonicalized to a load field, a runtime check has to be inserted in order to a add a
-     * g1-pre barrier if the loaded field is the referent field of the java.lang.ref.Reference
-     * class. The following code constructs the runtime check:
-     * 
-     * <pre>
-     * if (offset == referentOffset() && type == java.lang.ref.Reference) {
-     *     read;
-     *     G1PreWriteBarrier(read);
-     * } else {
-     *     read;
-     * }
-     * 
-     * </pre>
-     * 
-     * TODO (ck): Replace the code below with a snippet, properly fix the issue with the unsafe
-     * loads generated by the array copy intrinsics (robust assertions).
-     * 
-     */
-    private void lowerUnsafeLoad(UnsafeLoadNode load, LoweringTool tool) {
-        StructuredGraph graph = load.graph();
-        boolean compress = (!load.object().isNullConstant() && load.accessKind() == Kind.Object);
-        if (config().useG1GC && load.object().kind() == Kind.Object && load.accessKind() == Kind.Object && !load.object().objectStamp().alwaysNull() && load.object().objectStamp().type() != null &&
-                        !(load.object().objectStamp().type().isArray()) && tool.getLoweringType() != LoweringType.AFTER_GUARDS) {
-            IndexedLocationNode location = IndexedLocationNode.create(ANY_LOCATION, load.accessKind(), load.displacement(), load.offset(), graph, 1);
-            // Calculate offset+displacement
-            IntegerAddNode addNode = graph.add(new IntegerAddNode(Kind.Long, load.offset(), ConstantNode.forLong(load.displacement(), graph)));
-            // Compare previous result with referent offset (16)
-            CompareNode offsetCondition = CompareNode.createCompareNode(Condition.EQ, addNode, ConstantNode.forLong(referentOffset(), graph));
-            // Instance of unsafe load is java.lang.ref.Reference
-            InstanceOfNode instanceOfNode = graph.add(new InstanceOfNode(lookupJavaType(java.lang.ref.Reference.class), load.object(), null));
-            // The two barriers
-            ReadNode memoryReadNoBarrier = graph.add(new ReadNode(load.object(), location, load.stamp(), WriteBarrierType.NONE, compress));
-            ReadNode memoryReadBarrier = graph.add(new ReadNode(load.object(), location, load.stamp(), WriteBarrierType.PRECISE, compress));
-
-            // EndNodes
-            EndNode leftTrue = graph.add(new EndNode());
-            EndNode leftFalse = graph.add(new EndNode());
-            EndNode rightFirst = graph.add(new EndNode());
-            EndNode rightSecond = graph.add(new EndNode());
-
-            // MergeNodes
-            MergeNode mergeNoBarrier = graph.add(new MergeNode());
-            MergeNode mergeFinal = graph.add(new MergeNode());
-
-            // IfNodes
-            IfNode ifNodeType = graph.add(new IfNode(instanceOfNode, memoryReadBarrier, leftFalse, 0.1));
-            IfNode ifNodeOffset = graph.add(new IfNode(offsetCondition, ifNodeType, rightFirst, 0.1));
-
-            // Both branches are true (i.e. Add the barrier)
-            memoryReadBarrier.setNext(leftTrue);
-            mergeNoBarrier.addForwardEnd(rightFirst);
-            mergeNoBarrier.addForwardEnd(leftFalse);
-            mergeNoBarrier.setNext(memoryReadNoBarrier);
-            memoryReadNoBarrier.setNext(rightSecond);
-            mergeFinal.addForwardEnd(leftTrue);
-            mergeFinal.addForwardEnd(rightSecond);
-
-            PhiNode phiNode = graph.add(new PhiNode(load.accessKind(), mergeFinal));
-            phiNode.addInput(memoryReadBarrier);
-            phiNode.addInput(memoryReadNoBarrier);
-
-            // An unsafe read must not floating outside its block as may float above an explicit
-            // null check on its object.
-            memoryReadNoBarrier.setGuard(AbstractBeginNode.prevBegin(memoryReadNoBarrier));
-            memoryReadBarrier.setGuard(AbstractBeginNode.prevBegin(memoryReadBarrier));
-
-            assert load.successors().count() == 1;
-            Node next = load.successors().first();
-            load.replaceAndDelete(ifNodeOffset);
-            mergeFinal.setNext((FixedNode) next);
-            ifNodeOffset.replaceAtUsages(phiNode);
-        } else {
-            IndexedLocationNode location = IndexedLocationNode.create(ANY_LOCATION, load.accessKind(), load.displacement(), load.offset(), graph, 1);
-            // Unsafe access to a metaspace or to any
-            // absolute address do not perform uncompression.
-            ReadNode memoryRead = graph.add(new ReadNode(load.object(), location, load.stamp(), WriteBarrierType.NONE, compress));
-            // An unsafe read must not float outside its block otherwise
-            // it may float above an explicit null check on its object.
-            memoryRead.setGuard(AbstractBeginNode.prevBegin(load));
-            graph.replaceFixedWithFixed(load, memoryRead);
-        }
-    }
-
     private static WriteBarrierType getFieldLoadBarrierType(HotSpotResolvedJavaField loadField) {
         WriteBarrierType barrierType = WriteBarrierType.NONE;
         if (config().useG1GC && loadField.getKind() == Kind.Object && loadField.getDeclaringClass().mirror() == java.lang.ref.Reference.class && loadField.getName().equals("referent")) {
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/UnsafeLoadSnippets.java	Wed Jul 17 20:23:36 2013 +0200
@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2013, 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.
+ *
+ * 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 com.oracle.graal.hotspot.replacements;
+
+import static com.oracle.graal.hotspot.replacements.HotSpotReplacementsUtil.*;
+import static com.oracle.graal.replacements.SnippetTemplate.*;
+
+import com.oracle.graal.api.code.*;
+import com.oracle.graal.nodes.extended.*;
+import com.oracle.graal.nodes.spi.*;
+import com.oracle.graal.replacements.*;
+import com.oracle.graal.replacements.SnippetTemplate.AbstractTemplates;
+import com.oracle.graal.replacements.SnippetTemplate.Arguments;
+import com.oracle.graal.replacements.SnippetTemplate.SnippetInfo;
+import com.oracle.graal.word.*;
+
+public class UnsafeLoadSnippets implements Snippets {
+
+    @Snippet
+    public static Object lowerUnsafeLoad(Object object, long offset, int disp) {
+        long displacement = disp + offset;
+        if (object instanceof java.lang.ref.Reference && referentOffset() == displacement) {
+            return Word.fromObject(object).readObject((int) displacement, 1, true);
+        } else {
+            return Word.fromObject(object).readObject((int) displacement, 0, true);
+        }
+    }
+
+    public static class Templates extends AbstractTemplates {
+
+        private final SnippetInfo unsafeLoad = snippet(UnsafeLoadSnippets.class, "lowerUnsafeLoad");
+
+        public Templates(CodeCacheProvider runtime, Replacements replacements, TargetDescription target) {
+            super(runtime, replacements, target);
+        }
+
+        public void lower(UnsafeLoadNode load, @SuppressWarnings("unused") LoweringTool tool) {
+            Arguments args = new Arguments(unsafeLoad);
+            args.add("object", load.object());
+            args.add("offset", load.offset());
+            args.add("disp", load.displacement());
+            template(args).instantiate(runtime, load, DEFAULT_REPLACER, args);
+        }
+    }
+}