changeset 10637:ba1fbbfac0cd

remove null check semantics from LoadHubNode (GRAAL-248)
author Doug Simon <doug.simon@oracle.com>
date Fri, 05 Jul 2013 15:48:48 +0200
parents bef82f0cf71d
children 87c441b324e9
files graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotRuntime.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ArrayCopySnippets.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/HotSpotReplacementsUtil.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/MonitorSnippets.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/calc/IsNullNode.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/LoadHubNode.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningUtil.java
diffstat 7 files changed, 91 insertions(+), 70 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotRuntime.java	Fri Jul 05 15:47:31 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotRuntime.java	Fri Jul 05 15:48:48 2013 +0200
@@ -490,11 +490,13 @@
         } else if (n instanceof Invoke) {
             Invoke invoke = (Invoke) n;
             if (invoke.callTarget() instanceof MethodCallTargetNode) {
+
                 MethodCallTargetNode callTarget = (MethodCallTargetNode) invoke.callTarget();
                 NodeInputList<ValueNode> parameters = callTarget.arguments();
                 ValueNode receiver = parameters.size() <= 0 ? null : parameters.get(0);
+                GuardingNode receiverNullCheck = null;
                 if (!callTarget.isStatic() && receiver.kind() == Kind.Object && !receiver.objectStamp().nonNull()) {
-                    tool.createNullCheckGuard(invoke, receiver);
+                    receiverNullCheck = tool.createNullCheckGuard(invoke, receiver);
                 }
                 JavaType[] signature = MetaUtil.signatureToTypes(callTarget.targetMethod().getSignature(), callTarget.isStatic() ? null : callTarget.targetMethod().getDeclaringClass());
 
@@ -506,7 +508,12 @@
                         if (hsMethod.isInVirtualMethodTable()) {
                             int vtableEntryOffset = hsMethod.vtableEntryOffset();
                             assert vtableEntryOffset > 0;
-                            ReadNode hub = this.createReadHub(tool, graph, wordKind, receiver);
+                            ReadNode hub = createReadHub(graph, wordKind, receiver);
+
+                            if (receiverNullCheck != null) {
+                                hub.setGuard(receiverNullCheck);
+                            }
+
                             ReadNode metaspaceMethod = createReadVirtualMethod(graph, wordKind, hub, hsMethod);
                             // We use LocationNode.ANY_LOCATION for the reads that access the
                             // compiled code entry as HotSpot does not guarantee they are final
@@ -631,7 +638,10 @@
             LoadHubNode loadHub = (LoadHubNode) n;
             assert loadHub.kind() == wordKind;
             ValueNode object = loadHub.object();
-            ReadNode hub = createReadHub(tool, graph, wordKind, object);
+            ReadNode hub = createReadHub(graph, wordKind, object);
+            // A hub read must not float outside its block otherwise
+            // it may float above an explicit null check on its object.
+            hub.setGuard(AbstractBeginNode.prevBegin(loadHub));
             graph.replaceFixed(loadHub, hub);
         } else if (n instanceof LoadMethodNode) {
             LoadMethodNode loadMethodNode = (LoadMethodNode) n;
@@ -822,12 +832,10 @@
         return metaspaceMethod;
     }
 
-    private ReadNode createReadHub(LoweringTool tool, StructuredGraph graph, Kind wordKind, ValueNode object) {
+    private ReadNode createReadHub(StructuredGraph graph, Kind wordKind, ValueNode object) {
         LocationNode location = ConstantLocationNode.create(FINAL_LOCATION, wordKind, config.hubOffset, graph);
         assert !object.isConstant() || object.asConstant().isNull();
-        ReadNode hub = graph.add(new ReadNode(object, location, StampFactory.forKind(wordKind()), WriteBarrierType.NONE, false));
-        tool.createNullCheckGuard(hub, object);
-        return hub;
+        return graph.add(new ReadNode(object, location, StampFactory.forKind(wordKind()), WriteBarrierType.NONE, false));
     }
 
     public static long referentOffset() {
@@ -914,11 +922,11 @@
             ifNodeOffset.replaceAtUsages(phiNode);
         } else {
             IndexedLocationNode location = IndexedLocationNode.create(ANY_LOCATION, load.accessKind(), load.displacement(), load.offset(), graph, 1);
-            // Unsafe Accesses to the metaspace or to any
+            // 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 floating outside its block as may float above an explicit
-            // null check on its object.
+            // 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);
         }
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ArrayCopySnippets.java	Fri Jul 05 15:47:31 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ArrayCopySnippets.java	Fri Jul 05 15:48:48 2013 +0200
@@ -24,6 +24,8 @@
 
 import static com.oracle.graal.api.meta.LocationIdentity.*;
 import static com.oracle.graal.hotspot.replacements.HotSpotReplacementsUtil.*;
+import static com.oracle.graal.nodes.GuardingPiNode.*;
+import static com.oracle.graal.nodes.calc.IsNullNode.*;
 import static com.oracle.graal.phases.GraalOptions.*;
 import static com.oracle.graal.replacements.nodes.BranchProbabilityNode.*;
 
@@ -35,10 +37,13 @@
 import com.oracle.graal.graph.*;
 import com.oracle.graal.hotspot.nodes.*;
 import com.oracle.graal.nodes.*;
+import com.oracle.graal.nodes.calc.*;
 import com.oracle.graal.nodes.extended.*;
 import com.oracle.graal.nodes.java.*;
+import com.oracle.graal.nodes.type.*;
 import com.oracle.graal.phases.*;
 import com.oracle.graal.replacements.*;
+import com.oracle.graal.replacements.Snippet.Fold;
 import com.oracle.graal.replacements.nodes.*;
 import com.oracle.graal.word.*;
 
@@ -77,17 +82,19 @@
     private static final long VECTOR_SIZE = arrayIndexScale(Kind.Long);
 
     private static void checkedCopy(Object src, int srcPos, Object dest, int destPos, int length, Kind baseKind) {
-        checkNonNull(src);
-        checkNonNull(dest);
-        checkLimits(src, srcPos, dest, destPos, length);
-        UnsafeArrayCopyNode.arraycopy(src, srcPos, dest, destPos, length, baseKind);
+        Object nonNullSrc = checkNonNull(src);
+        Object nonNullDest = checkNonNull(dest);
+        checkLimits(nonNullSrc, srcPos, nonNullDest, destPos, length);
+        UnsafeArrayCopyNode.arraycopy(nonNullSrc, srcPos, nonNullDest, destPos, length, baseKind);
     }
 
-    public static void checkNonNull(Object obj) {
-        if (obj == null) {
-            checkNPECounter.inc();
-            DeoptimizeNode.deopt(DeoptimizationAction.None, DeoptimizationReason.RuntimeConstraint);
-        }
+    @Fold
+    private static Stamp nonNull() {
+        return StampFactory.objectNonNull();
+    }
+
+    public static Object checkNonNull(Object obj) {
+        return guardingPi(obj, isNull(obj), true, DeoptimizationReason.RuntimeConstraint, DeoptimizationAction.None, nonNull());
     }
 
     public static int checkArrayType(Word hub) {
@@ -184,24 +191,25 @@
 
     @Snippet
     public static void arraycopy(Object src, int srcPos, Object dest, int destPos, int length) {
-        // loading the hubs also checks for nullness
-        Word srcHub = loadHub(src);
-        Word destHub = loadHub(dest);
-        if (probability(FAST_PATH_PROBABILITY, srcHub.equal(destHub)) && probability(FAST_PATH_PROBABILITY, src != dest)) {
+        Object nonNullSrc = checkNonNull(src);
+        Object nonNullDest = checkNonNull(dest);
+        Word srcHub = loadHub(nonNullSrc);
+        Word destHub = loadHub(nonNullDest);
+        if (probability(FAST_PATH_PROBABILITY, srcHub.equal(destHub)) && probability(FAST_PATH_PROBABILITY, nonNullSrc != nonNullDest)) {
             int layoutHelper = checkArrayType(srcHub);
             final boolean isObjectArray = ((layoutHelper & layoutHelperElementTypePrimitiveInPlace()) == 0);
 
-            checkLimits(src, srcPos, dest, destPos, length);
+            checkLimits(nonNullSrc, srcPos, nonNullDest, destPos, length);
             if (probability(FAST_PATH_PROBABILITY, isObjectArray)) {
                 genericObjectExactCallCounter.inc();
-                arrayObjectCopy(src, srcPos, dest, destPos, length);
+                arrayObjectCopy(nonNullSrc, srcPos, nonNullDest, destPos, length);
             } else {
                 genericPrimitiveCallCounter.inc();
-                UnsafeArrayCopyNode.arraycopyPrimitive(src, srcPos, dest, destPos, length, layoutHelper);
+                UnsafeArrayCopyNode.arraycopyPrimitive(nonNullSrc, srcPos, nonNullDest, destPos, length, layoutHelper);
             }
         } else {
             genericObjectCallCounter.inc();
-            System.arraycopy(src, srcPos, dest, destPos, length);
+            System.arraycopy(nonNullSrc, srcPos, nonNullDest, destPos, length);
         }
     }
 
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/HotSpotReplacementsUtil.java	Fri Jul 05 15:47:31 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/HotSpotReplacementsUtil.java	Fri Jul 05 15:48:48 2013 +0200
@@ -437,19 +437,12 @@
     }
 
     /**
-     * Loads the hub from a object, null checking it first.
+     * Loads the hub of an object (without null checking it first).
      */
     public static Word loadHub(Object object) {
         return loadHubIntrinsic(object, getWordKind());
     }
 
-    /**
-     * Loads the hub from a object.
-     */
-    public static Word loadHubNoNullcheck(Object object) {
-        return loadWordFromObject(object, hubOffset());
-    }
-
     public static Object verifyOop(Object object) {
         if (verifyOops()) {
             verifyOopStub(VERIFY_OOP, object);
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/MonitorSnippets.java	Fri Jul 05 15:47:31 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/MonitorSnippets.java	Fri Jul 05 15:48:48 2013 +0200
@@ -107,7 +107,7 @@
             } else {
                 // The bias pattern is present in the object's mark word. Need to check
                 // whether the bias owner and the epoch are both still current.
-                Word hub = loadHubNoNullcheck(object);
+                Word hub = loadHub(object);
                 final Word prototypeMarkWord = hub.readWord(prototypeMarkWordOffset(), PROTOTYPE_MARK_WORD_LOCATION);
                 final Word thread = thread();
                 final Word tmp = prototypeMarkWord.or(thread).xor(mark).and(~ageMaskInPlace());
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/calc/IsNullNode.java	Fri Jul 05 15:47:31 2013 +0200
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/calc/IsNullNode.java	Fri Jul 05 15:48:48 2013 +0200
@@ -92,4 +92,7 @@
         }
         return false;
     }
+
+    @NodeIntrinsic
+    public static native IsNullNode isNull(Object object);
 }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/LoadHubNode.java	Fri Jul 05 15:47:31 2013 +0200
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/LoadHubNode.java	Fri Jul 05 15:48:48 2013 +0200
@@ -29,7 +29,8 @@
 import com.oracle.graal.nodes.type.*;
 
 /**
- * Loads an object's {@linkplain Representation#ObjectHub hub}, null-checking the object first.
+ * Loads an object's {@linkplain Representation#ObjectHub hub}. The object is not null-checked by
+ * this operation.
  */
 public final class LoadHubNode extends FixedWithNextNode implements Lowerable, Canonicalizable, Virtualizable {
 
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningUtil.java	Fri Jul 05 15:47:31 2013 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningUtil.java	Fri Jul 05 15:48:48 2013 +0200
@@ -22,6 +22,9 @@
  */
 package com.oracle.graal.phases.common;
 
+import static com.oracle.graal.api.code.DeoptimizationAction.*;
+import static com.oracle.graal.api.meta.DeoptimizationReason.*;
+import static com.oracle.graal.nodes.type.StampFactory.*;
 import static com.oracle.graal.phases.GraalOptions.*;
 
 import java.lang.reflect.*;
@@ -251,18 +254,26 @@
     }
 
     /**
-     * Represents an opportunity for inlining at the given invoke, with the given weight and level.
+     * Represents an opportunity for inlining at a given invoke, with the given weight and level.
      * The weight is the amortized weight of the additional code - so smaller is better. The level
      * is the number of nested inlinings that lead to this invoke.
      */
     public interface InlineInfo {
 
+        /**
+         * The graph containing the {@link #invoke() invocation} that may be inlined.
+         */
         StructuredGraph graph();
 
+        /**
+         * The invocation that may be inlined.
+         */
         Invoke invoke();
 
         /**
-         * Returns the number of invoked methods.
+         * Returns the number of methods that may be inlined by the {@link #invoke() invocation}.
+         * This may be more than one in the case of a invocation profile showing a number of "hot"
+         * concrete methods dispatched to by the invocation.
          */
         int numberOfMethods();
 
@@ -485,16 +496,16 @@
         }
 
         private void createGuard(StructuredGraph graph, MetaAccessProvider runtime) {
-            InliningUtil.receiverNullCheck(invoke);
-            ValueNode receiver = ((MethodCallTargetNode) invoke.callTarget()).receiver();
+            ValueNode nonNullReceiver = InliningUtil.nonNullReceiver(invoke);
             ConstantNode typeHub = ConstantNode.forConstant(type.getEncoding(Representation.ObjectHub), runtime, graph);
-            LoadHubNode receiverHub = graph.add(new LoadHubNode(receiver, typeHub.kind()));
+            LoadHubNode receiverHub = graph.add(new LoadHubNode(nonNullReceiver, typeHub.kind()));
+
             CompareNode typeCheck = CompareNode.createCompareNode(Condition.EQ, receiverHub, typeHub);
             FixedGuardNode guard = graph.add(new FixedGuardNode(typeCheck, DeoptimizationReason.TypeCheckedInliningViolated, DeoptimizationAction.InvalidateReprofile));
             assert invoke.predecessor() != null;
 
-            ValueNode anchoredReceiver = createAnchoredReceiver(graph, guard, type, receiver, true);
-            invoke.callTarget().replaceFirstInput(receiver, anchoredReceiver);
+            ValueNode anchoredReceiver = createAnchoredReceiver(graph, guard, type, nonNullReceiver, true);
+            invoke.callTarget().replaceFirstInput(nonNullReceiver, anchoredReceiver);
 
             graph.addBeforeFixed(invoke.asNode(), receiverHub);
             graph.addBeforeFixed(invoke.asNode(), guard);
@@ -508,7 +519,7 @@
 
     /**
      * Polymorphic inlining of m methods with n type checks (n >= m) in case that the profiling
-     * information suggests a reasonable amounts of different receiver types and different methods.
+     * information suggests a reasonable amount of different receiver types and different methods.
      * If an unknown type is encountered a deoptimization is triggered.
      */
     private static class MultiTypeGuardInlineInfo extends AbstractInlineInfo {
@@ -592,8 +603,6 @@
 
         @Override
         public void inline(MetaAccessProvider runtime, Assumptions assumptions, Replacements replacements) {
-            // receiver null check must be the first node
-            InliningUtil.receiverNullCheck(invoke);
             if (hasSingleMethod()) {
                 inlineSingleMethod(graph(), runtime, assumptions);
             } else {
@@ -773,9 +782,9 @@
 
         private boolean createDispatchOnTypeBeforeInvoke(StructuredGraph graph, AbstractBeginNode[] successors, boolean invokeIsOnlySuccessor, MetaAccessProvider runtime) {
             assert ptypes.size() >= 1;
-
+            ValueNode nonNullReceiver = nonNullReceiver(invoke);
             Kind hubKind = ((MethodCallTargetNode) invoke.callTarget()).targetMethod().getDeclaringClass().getEncoding(Representation.ObjectHub).getKind();
-            LoadHubNode hub = graph.add(new LoadHubNode(((MethodCallTargetNode) invoke.callTarget()).receiver(), hubKind));
+            LoadHubNode hub = graph.add(new LoadHubNode(nonNullReceiver, hubKind));
             graph.addBeforeFixed(invoke.asNode(), hub);
 
             if (!invokeIsOnlySuccessor && chooseMethodDispatch()) {
@@ -960,8 +969,6 @@
         }
 
         private void devirtualizeWithTypeSwitch(StructuredGraph graph, InvokeKind kind, ResolvedJavaMethod target, MetaAccessProvider runtime) {
-            InliningUtil.receiverNullCheck(invoke);
-
             AbstractBeginNode invocationEntry = graph.add(new BeginNode());
             AbstractBeginNode unknownTypeSux = createUnknownTypeSuccessor(graph);
             AbstractBeginNode[] successors = new AbstractBeginNode[]{invocationEntry, unknownTypeSux};
@@ -1052,6 +1059,10 @@
 
         ResolvedJavaType holder = targetMethod.getDeclaringClass();
         ObjectStamp receiverStamp = callTarget.receiver().objectStamp();
+        if (receiverStamp.alwaysNull()) {
+            // Don't inline if receiver is known to be null
+            return null;
+        }
         if (receiverStamp.type() != null) {
             // the invoke target might be more specific than the holder (happens after inlining:
             // locals lose their declared type...)
@@ -1300,17 +1311,8 @@
 
         FrameState stateAfter = invoke.stateAfter();
         assert stateAfter == null || stateAfter.isAlive();
-        if (receiverNullCheck) {
-            GuardingNode receiverNullCheckNode = receiverNullCheck(invoke);
-            if (receiverNullCheckNode != null) {
-                ValueNode receiver = invoke.callTarget().arguments().get(0);
-                Stamp piStamp = receiver.stamp();
-                if (piStamp instanceof ObjectStamp) {
-                    piStamp = piStamp.join(StampFactory.objectNonNull());
-                }
-                ValueNode anchoredReceiver = createAnchoredReceiver(graph, receiverNullCheckNode, receiver, piStamp);
-                invoke.callTarget().replaceFirstInput(receiver, anchoredReceiver);
-            }
+        if (receiverNullCheck && !((MethodCallTargetNode) invoke.callTarget()).isStatic()) {
+            nonNullReceiver(invoke);
         }
 
         IdentityHashMap<Node, Node> replacements = new IdentityHashMap<>();
@@ -1448,17 +1450,23 @@
         return true;
     }
 
-    public static GuardingNode receiverNullCheck(Invoke invoke) {
+    /**
+     * Gets the receiver for an invoke, adding a guard if necessary to ensure it is non-null.
+     */
+    public static ValueNode nonNullReceiver(Invoke invoke) {
         MethodCallTargetNode callTarget = (MethodCallTargetNode) invoke.callTarget();
+        assert !callTarget.isStatic() : callTarget.targetMethod();
         StructuredGraph graph = callTarget.graph();
-        NodeInputList<ValueNode> parameters = callTarget.arguments();
-        ValueNode firstParam = parameters.size() <= 0 ? null : parameters.get(0);
-        if (!callTarget.isStatic() && firstParam.kind() == Kind.Object && !firstParam.objectStamp().nonNull()) {
-            FixedGuardNode guard = graph.add(new FixedGuardNode(graph.unique(new IsNullNode(firstParam)), DeoptimizationReason.NullCheckException, DeoptimizationAction.InvalidateReprofile, true));
-            graph.addBeforeFixed(invoke.asNode(), guard);
-            return guard;
+        ValueNode firstParam = callTarget.arguments().get(0);
+        if (firstParam.kind() == Kind.Object && !firstParam.objectStamp().nonNull()) {
+            assert !firstParam.objectStamp().alwaysNull();
+            IsNullNode condition = graph.unique(new IsNullNode(firstParam));
+            GuardingPiNode nonNullReceiver = graph.add(new GuardingPiNode(firstParam, condition, true, NullCheckException, InvalidateReprofile, objectNonNull()));
+            graph.addBeforeFixed(invoke.asNode(), nonNullReceiver);
+            callTarget.replaceFirstInput(firstParam, nonNullReceiver);
+            return nonNullReceiver;
         }
-        return null;
+        return firstParam;
     }
 
     public static boolean canIntrinsify(Replacements replacements, ResolvedJavaMethod target) {