Introduce mayNullCheckSkipConversion method in ConvertNode interface.…

… Move address lowering after fixed read phase.
ZhangTongLe · Mar 13, 2017 · 6f36b03 · 6f36b03
1 parent 74fe000
commit 6f36b03
Show file tree

Hide file tree

Showing 10 changed files with 58 additions and 5 deletions.
diff --git a/...graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotSuitesProvider.java b/...graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotSuitesProvider.java
@@ -56,10 +56,12 @@
 import org.graalvm.compiler.phases.common.AddressLoweringPhase.AddressLowering;
 import org.graalvm.compiler.phases.common.CanonicalizerPhase;
 import org.graalvm.compiler.phases.common.ExpandLogicPhase;
+import org.graalvm.compiler.phases.common.FixReadsPhase;
 import org.graalvm.compiler.phases.common.LoopSafepointInsertionPhase;
 import org.graalvm.compiler.phases.common.LoweringPhase;
 import org.graalvm.compiler.phases.common.inlining.InliningPhase;
 import org.graalvm.compiler.phases.tiers.HighTierContext;
+import org.graalvm.compiler.phases.tiers.LowTierContext;
 import org.graalvm.compiler.phases.tiers.Suites;
 import org.graalvm.compiler.phases.tiers.SuitesCreator;
 
@@ -115,7 +117,11 @@ public Suites createSuites(OptionValues options) {
             ret.getMidTier().appendPhase(new WriteBarrierVerificationPhase(config));
         }
 
-        ret.getLowTier().findPhase(ExpandLogicPhase.class).add(new AddressLoweringPhase(addressLowering));
+        ListIterator<BasePhase<? super LowTierContext>> findPhase = ret.getLowTier().findPhase(FixReadsPhase.class);
+        if (findPhase == null) {
+            findPhase = ret.getLowTier().findPhase(ExpandLogicPhase.class);
+        }
+        findPhase.add(new AddressLoweringPhase(addressLowering));
 
         return ret;
     }

diff --git a/.../org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/nodes/CompressionNode.java b/.../org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/nodes/CompressionNode.java
@@ -214,6 +214,12 @@ public void generate(NodeLIRBuilderTool gen) {
             default:
                 throw GraalError.shouldNotReachHere();
         }
+
         gen.setResult(this, result);
     }
+
+    @Override
+    public boolean mayNullCheckSkipConversion() {
+        return true;
+    }
 }
diff --git a/...aalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/replacements/ClassGetHubNode.java b/...aalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/replacements/ClassGetHubNode.java
@@ -135,6 +135,14 @@ public boolean isLossless() {
         return false;
     }
 
+    /**
+     * There is more than one {@link java.lang.Class} value that has a NULL hub.
+     */
+    @Override
+    public boolean mayNullCheckSkipConversion() {
+        return false;
+    }
+
     @Override
     public boolean preservesOrder(Condition op, Constant value, ConstantReflectionProvider constantReflection) {
         assert op == Condition.EQ || op == Condition.NE;

diff --git a/...aalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/replacements/HubGetClassNode.java b/...aalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/replacements/HubGetClassNode.java
@@ -114,11 +114,16 @@ public Constant reverse(Constant c, ConstantReflectionProvider constantReflectio
         }
     }
 
+    /**
+     * Any concrete Klass* has a corresponding {@link java.lang.Class}.
+     */
     @Override
     public boolean isLossless() {
-        /*
-         * Any concrete Klass* has a corresponding java.lang.Class
-         */
+        return true;
+    }
+
+    @Override
+    public boolean mayNullCheckSkipConversion() {
         return true;
     }
 }
diff --git a/graal/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/ConvertNode.java b/graal/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/ConvertNode.java
@@ -40,6 +40,14 @@ public interface ConvertNode extends ValueNodeInterface {
 
     Constant reverse(Constant c, ConstantReflectionProvider constantReflection);
 
+    /**
+     * Checks whether a null check may skip the conversion. This is true if in the conversion NULL
+     * is converted to NULL and if it is the only value converted to NULL.
+     *
+     * @return whether a null check may skip the conversion
+     */
+    boolean mayNullCheckSkipConversion();
+
     /**
      * Check whether a conversion is lossless.
      *

diff --git a/graal/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/FloatConvertNode.java b/graal/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/FloatConvertNode.java
@@ -124,4 +124,9 @@ public void lower(LoweringTool tool) {
     public void generate(NodeLIRBuilderTool nodeValueMap, ArithmeticLIRGeneratorTool gen) {
         nodeValueMap.setResult(this, gen.emitFloatConvert(getFloatConvert(), nodeValueMap.operand(getValue())));
     }
+
+    @Override
+    public boolean mayNullCheckSkipConversion() {
+        return false;
+    }
 }
diff --git a/graal/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/IsNullNode.java b/graal/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/IsNullNode.java
@@ -96,7 +96,7 @@ public ValueNode canonical(CanonicalizerTool tool, ValueNode forValue) {
 
         if (forValue instanceof ConvertNode) {
             ConvertNode convertNode = (ConvertNode) forValue;
-            if (convertNode.isLossless()) {
+            if (convertNode.mayNullCheckSkipConversion()) {
                 return IsNullNode.create(convertNode.getValue());
             }
         }

diff --git a/graal/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/NarrowNode.java b/graal/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/NarrowNode.java
@@ -120,4 +120,9 @@ public ValueNode canonical(CanonicalizerTool tool, ValueNode forValue) {
     public void generate(NodeLIRBuilderTool nodeValueMap, ArithmeticLIRGeneratorTool gen) {
         nodeValueMap.setResult(this, gen.emitNarrow(nodeValueMap.operand(getValue()), getResultBits()));
     }
+
+    @Override
+    public boolean mayNullCheckSkipConversion() {
+        return false;
+    }
 }
diff --git a/graal/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/SignExtendNode.java b/graal/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/SignExtendNode.java
@@ -110,4 +110,9 @@ public ValueNode canonical(CanonicalizerTool tool, ValueNode forValue) {
     public void generate(NodeLIRBuilderTool nodeValueMap, ArithmeticLIRGeneratorTool gen) {
         nodeValueMap.setResult(this, gen.emitSignExtend(nodeValueMap.operand(getValue()), getInputBits(), getResultBits()));
     }
+
+    @Override
+    public boolean mayNullCheckSkipConversion() {
+        return true;
+    }
 }
diff --git a/graal/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/ZeroExtendNode.java b/graal/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/ZeroExtendNode.java
@@ -124,4 +124,9 @@ public ValueNode canonical(CanonicalizerTool tool, ValueNode forValue) {
     public void generate(NodeLIRBuilderTool nodeValueMap, ArithmeticLIRGeneratorTool gen) {
         nodeValueMap.setResult(this, gen.emitZeroExtend(nodeValueMap.operand(getValue()), getInputBits(), getResultBits()));
     }
+
+    @Override
+    public boolean mayNullCheckSkipConversion() {
+        return true;
+    }
 }