[GR-2872] Improve handling of PiNodes in address expressions.

ZhangTongLe · Feb 15, 2017 · 7d7bc36 · 7d7bc36
2 parents 7a18b1d + 0c9f8cd
commit 7d7bc36
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 6 deletions.
diff --git a/graal/org.graalvm.compiler.core/src/org/graalvm/compiler/core/match/MatchContext.java b/graal/org.graalvm.compiler.core/src/org/graalvm/compiler/core/match/MatchContext.java
@@ -151,7 +151,7 @@ public void setResult(ComplexMatchResult result) {
      * @return Result.OK if the node can be safely consumed.
      */
     public Result consume(Node node) {
-        assert node.getUsageCount() <= 1 : "should have already been checked";
+        assert MatchPattern.isSingleValueUser(node) : "should have already been checked";
 
         // Check NOT_IN_BLOCK first since that usually implies ALREADY_USED
         int index = nodes.indexOf(node);

diff --git a/graal/org.graalvm.compiler.core/src/org/graalvm/compiler/core/match/MatchPattern.java b/graal/org.graalvm.compiler.core/src/org/graalvm/compiler/core/match/MatchPattern.java
@@ -26,6 +26,7 @@
 import org.graalvm.compiler.debug.DebugCounter;
 import org.graalvm.compiler.graph.Node;
 import org.graalvm.compiler.graph.Position;
+import org.graalvm.compiler.nodeinfo.InputType;
 import org.graalvm.compiler.nodeinfo.Verbosity;
 
 /**
@@ -252,7 +253,7 @@ private Result matchShape(Node node, MatchStatement statement, boolean atRoot) {
         }
 
         if (singleUser && !atRoot) {
-            if (node.getUsageCount() > 1) {
+            if (!isSingleValueUser(node)) {
                 return Result.tooManyUsers(node, statement.getPattern());
             }
         }
@@ -267,6 +268,27 @@ private Result matchShape(Node node, MatchStatement statement, boolean atRoot) {
         return result;
     }
 
+    public static boolean isSingleValueUser(Node node) {
+        int valueUsage = node.getUsageCount();
+        if (valueUsage == 1) {
+            return true;
+        }
+        if (node.isAllowedUsageType(InputType.Guard)) {
+            valueUsage = 0;
+            for (Node usage : node.usages()) {
+                for (Position input : usage.inputPositions()) {
+                    if (input.getInputType() == InputType.Value && input.get(usage) == node) {
+                        valueUsage++;
+                        if (valueUsage > 1) {
+                            return false;
+                        }
+                    }
+                }
+            }
+        }
+        return false;
+    }
+
     /**
      * For a node starting at root, produce a String showing the inputs that matched against this
      * rule. It's assumed that a match has already succeeded against this rule, otherwise the

diff --git a/...ler.hotspot.amd64/src/org/graalvm/compiler/hotspot/amd64/AMD64HotSpotAddressLowering.java b/...ler.hotspot.amd64/src/org/graalvm/compiler/hotspot/amd64/AMD64HotSpotAddressLowering.java
@@ -32,7 +32,9 @@
 import org.graalvm.compiler.core.amd64.AMD64AddressLowering;
 import org.graalvm.compiler.core.amd64.AMD64AddressNode;
 import org.graalvm.compiler.core.common.LIRKind;
+import org.graalvm.compiler.core.common.type.AbstractObjectStamp;
 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.NodeClass;
 import org.graalvm.compiler.hotspot.CompressEncoding;
@@ -41,7 +43,9 @@
 import org.graalvm.compiler.hotspot.nodes.CompressionNode.CompressionOp;
 import org.graalvm.compiler.hotspot.nodes.GraalHotSpotVMConfigNode;
 import org.graalvm.compiler.hotspot.nodes.type.KlassPointerStamp;
+import org.graalvm.compiler.hotspot.nodes.type.NarrowOopStamp;
 import org.graalvm.compiler.nodeinfo.NodeInfo;
+import org.graalvm.compiler.nodes.PiNode;
 import org.graalvm.compiler.nodes.ValueNode;
 import org.graalvm.compiler.nodes.calc.FloatingNode;
 import org.graalvm.compiler.nodes.spi.LIRLowerable;
@@ -102,6 +106,28 @@ protected boolean improve(AMD64AddressNode addr) {
                     return true;
                 }
             }
+
+            if (addr.getIndex() == null && addr.getBase() instanceof PiNode) {
+                PiNode pi = (PiNode) addr.getBase();
+                if (pi.getOriginalNode() instanceof CompressionNode) {
+                    CompressionNode compression = (CompressionNode) pi.getOriginalNode();
+                    // Strip out the Pi and see if the compression can fold
+                    addr.setBase(compression);
+                    if (improveUncompression(addr, compression)) {
+                        /*
+                         * Move the pi onto the other side of the compression.
+                         */
+                        assert addr.getIndex() == compression.getValue();
+                        Stamp newStamp = NarrowOopStamp.compressed((AbstractObjectStamp) pi.stamp(), compression.getEncoding());
+                        PiNode newPi = compression.graph().unique(new PiNode(addr.getIndex(), newStamp, pi.getGuard() != null ? pi.getGuard().asNode() : null));
+                        addr.setIndex(newPi);
+                        return true;
+                    } else {
+                        // Can't fold the compression so restore the base
+                        addr.setBase(pi);
+                    }
+                }
+            }
         }
 
         return super.improve(addr);

diff --git a/...piler.replacements/src/org/graalvm/compiler/replacements/DefaultJavaLoweringProvider.java b/...piler.replacements/src/org/graalvm/compiler/replacements/DefaultJavaLoweringProvider.java
@@ -367,6 +367,16 @@ protected void lowerStoreFieldNode(StoreFieldNode storeField, LoweringTool tool)
         }
     }
 
+    /**
+     * Create a PiNode on the index proving that the index is positive. On some platforms this is
+     * important to allow the index to be used as an int in the address mode.
+     */
+    public AddressNode createArrayIndexAddress(StructuredGraph graph, ValueNode array, JavaKind elementKind, ValueNode index, GuardingNode boundsCheck) {
+        IntegerStamp indexStamp = StampFactory.forInteger(32, 0, Integer.MAX_VALUE - 1);
+        ValueNode positiveIndex = graph.unique(new PiNode(index, indexStamp, boundsCheck != null ? boundsCheck.asNode() : null));
+        return createArrayAddress(graph, array, elementKind, positiveIndex);
+    }
+
     public AddressNode createArrayAddress(StructuredGraph graph, ValueNode array, JavaKind elementKind, ValueNode index) {
         ValueNode wordIndex;
         if (target.wordSize > 4) {
@@ -393,7 +403,7 @@ protected void lowerLoadIndexedNode(LoadIndexedNode loadIndexed, LoweringTool to
         Stamp loadStamp = loadStamp(loadIndexed.stamp(), elementKind);
 
         GuardingNode boundsCheck = getBoundsCheck(loadIndexed, array, tool);
-        AddressNode address = createArrayAddress(graph, array, elementKind, loadIndexed.index());
+        AddressNode address = createArrayIndexAddress(graph, array, elementKind, loadIndexed.index(), boundsCheck);
         ReadNode memoryRead = graph.add(new ReadNode(address, NamedLocationIdentity.getArrayLocation(elementKind), loadStamp, boundsCheck, BarrierType.NONE));
         ValueNode readValue = implicitLoadConvert(graph, elementKind, memoryRead);
 
@@ -409,7 +419,7 @@ protected void lowerStoreIndexedNode(StoreIndexedNode storeIndexed, LoweringTool
 
         array = this.createNullCheckedValue(array, storeIndexed, tool);
 
-        GuardingNode checkedIndex = getBoundsCheck(storeIndexed, array, tool);
+        GuardingNode boundsCheck = getBoundsCheck(storeIndexed, array, tool);
 
         JavaKind elementKind = storeIndexed.elementKind();
 
@@ -436,9 +446,9 @@ protected void lowerStoreIndexedNode(StoreIndexedNode storeIndexed, LoweringTool
             }
         }
 
-        AddressNode address = createArrayAddress(graph, array, elementKind, storeIndexed.index());
+        AddressNode address = createArrayIndexAddress(graph, array, elementKind, storeIndexed.index(), boundsCheck);
         WriteNode memoryWrite = graph.add(new WriteNode(address, NamedLocationIdentity.getArrayLocation(elementKind), implicitStoreConvert(graph, elementKind, value),
-                        arrayStoreBarrierType(storeIndexed.elementKind()), checkedIndex, false));
+                        arrayStoreBarrierType(storeIndexed.elementKind()), boundsCheck, false));
         if (condition != null) {
             GuardingNode storeCheckGuard = tool.createGuard(storeIndexed, condition, DeoptimizationReason.ArrayStoreException, DeoptimizationAction.InvalidateReprofile);
             memoryWrite.setStoreCheckGuard(storeCheckGuard);