changeset 8981:09cdf7224794

PEA: cleanup and documentation for recent changes
author Lukas Stadler <lukas.stadler@jku.at>
date Wed, 10 Apr 2013 10:17:35 +0200
parents 92b00825c037
children 39f088ba89a9
files graal/com.oracle.graal.api.replacements/src/com/oracle/graal/api/replacements/MethodSubstitution.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/BoxingSubstitutions.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/calc/ObjectEqualsNode.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/BoxNode.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/UnboxNode.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/spi/MacroSubstitution.java graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/PartialEscapeClosure.java
diffstat 8 files changed, 44 insertions(+), 46 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.api.replacements/src/com/oracle/graal/api/replacements/MethodSubstitution.java	Wed Apr 10 02:10:14 2013 +0200
+++ b/graal/com.oracle.graal.api.replacements/src/com/oracle/graal/api/replacements/MethodSubstitution.java	Wed Apr 10 10:17:35 2013 +0200
@@ -58,8 +58,11 @@
     /**
      * Determines if this method should be substituted in all cases, even if inlining thinks it is
      * not important.
+     * 
+     * Not that this is still depending on whether inlining sees the correct call target, so it's
+     * only a hard guarantee for static and special invocations.
      */
-    boolean isForcedInlining() default false;
+    boolean forced() default false;
 
     /**
      * Determines if the substitution is for a method that may not be part of the runtime. For
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/BoxingSubstitutions.java	Wed Apr 10 02:10:14 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/BoxingSubstitutions.java	Wed Apr 10 10:17:35 2013 +0200
@@ -31,12 +31,12 @@
     @ClassSubstitution(Boolean.class)
     private static class BooleanSubstitutions {
 
-        @MethodSubstitution(isForcedInlining = true)
+        @MethodSubstitution(forced = true)
         public static Boolean valueOf(boolean value) {
             return BoxNode.box(value, Boolean.class, Kind.Boolean);
         }
 
-        @MethodSubstitution(isStatic = false, isForcedInlining = true)
+        @MethodSubstitution(isStatic = false, forced = true)
         public static boolean booleanValue(Boolean value) {
             return UnboxNode.unbox(value, Kind.Boolean);
         }
@@ -45,12 +45,12 @@
     @ClassSubstitution(Byte.class)
     private static class ByteSubstitutions {
 
-        @MethodSubstitution(isForcedInlining = true)
+        @MethodSubstitution(forced = true)
         public static Byte valueOf(byte value) {
             return BoxNode.box(value, Byte.class, Kind.Byte);
         }
 
-        @MethodSubstitution(isStatic = false, isForcedInlining = true)
+        @MethodSubstitution(isStatic = false, forced = true)
         public static byte byteValue(Byte value) {
             return UnboxNode.unbox(value, Kind.Byte);
         }
@@ -59,12 +59,12 @@
     @ClassSubstitution(Character.class)
     private static class CharacterSubstitutions {
 
-        @MethodSubstitution(isForcedInlining = true)
+        @MethodSubstitution(forced = true)
         public static Character valueOf(char value) {
             return BoxNode.box(value, Character.class, Kind.Char);
         }
 
-        @MethodSubstitution(isStatic = false, isForcedInlining = true)
+        @MethodSubstitution(isStatic = false, forced = true)
         public static char charValue(Character value) {
             return UnboxNode.unbox(value, Kind.Char);
         }
@@ -73,12 +73,12 @@
     @ClassSubstitution(Double.class)
     private static class DoubleSubstitutions {
 
-        @MethodSubstitution(isForcedInlining = true)
+        @MethodSubstitution(forced = true)
         public static Double valueOf(double value) {
             return BoxNode.box(value, Double.class, Kind.Double);
         }
 
-        @MethodSubstitution(isStatic = false, isForcedInlining = true)
+        @MethodSubstitution(isStatic = false, forced = true)
         public static double doubleValue(Double value) {
             return UnboxNode.unbox(value, Kind.Double);
         }
@@ -87,12 +87,12 @@
     @ClassSubstitution(Float.class)
     private static class FloatSubstitutions {
 
-        @MethodSubstitution(isForcedInlining = true)
+        @MethodSubstitution(forced = true)
         public static Float valueOf(float value) {
             return BoxNode.box(value, Float.class, Kind.Float);
         }
 
-        @MethodSubstitution(isStatic = false, isForcedInlining = true)
+        @MethodSubstitution(isStatic = false, forced = true)
         public static float floatValue(Float value) {
             return UnboxNode.unbox(value, Kind.Float);
         }
@@ -101,12 +101,12 @@
     @ClassSubstitution(Integer.class)
     private static class IntegerSubstitutions {
 
-        @MethodSubstitution(isForcedInlining = true)
+        @MethodSubstitution(forced = true)
         public static Integer valueOf(int value) {
             return BoxNode.box(value, Integer.class, Kind.Int);
         }
 
-        @MethodSubstitution(isStatic = false, isForcedInlining = true)
+        @MethodSubstitution(isStatic = false, forced = true)
         public static int intValue(Integer value) {
             return UnboxNode.unbox(value, Kind.Int);
         }
@@ -115,12 +115,12 @@
     @ClassSubstitution(Long.class)
     private static class LongSubstitutions {
 
-        @MethodSubstitution(isForcedInlining = true)
+        @MethodSubstitution(forced = true)
         public static Long valueOf(long value) {
             return BoxNode.box(value, Long.class, Kind.Long);
         }
 
-        @MethodSubstitution(isStatic = false, isForcedInlining = true)
+        @MethodSubstitution(isStatic = false, forced = true)
         public static long longValue(Long value) {
             return UnboxNode.unbox(value, Kind.Long);
         }
@@ -129,12 +129,12 @@
     @ClassSubstitution(Short.class)
     private static class ShortSubstitutions {
 
-        @MethodSubstitution(isForcedInlining = true)
+        @MethodSubstitution(forced = true)
         public static Short valueOf(short value) {
             return BoxNode.box(value, Short.class, Kind.Short);
         }
 
-        @MethodSubstitution(isStatic = false, isForcedInlining = true)
+        @MethodSubstitution(isStatic = false, forced = true)
         public static short shortValue(Short value) {
             return UnboxNode.unbox(value, Kind.Short);
         }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/calc/ObjectEqualsNode.java	Wed Apr 10 02:10:14 2013 +0200
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/calc/ObjectEqualsNode.java	Wed Apr 10 10:17:35 2013 +0200
@@ -84,6 +84,13 @@
             boolean xIdentity = stateX.getVirtualObject().hasIdentity();
             boolean yIdentity = stateY.getVirtualObject().hasIdentity();
             if (xIdentity ^ yIdentity) {
+                /*
+                 * One of the two objects has identity, the other doesn't. In code, this looks like
+                 * "Integer.valueOf(a) == new Integer(b)", which is always false.
+                 * 
+                 * In other words: an object created via valueOf can never be equal to one created
+                 * by new in the same compilation unit.
+                 */
                 tool.replaceWithValue(LogicConstantNode.contradiction(graph()));
             } else if (!xIdentity && !yIdentity) {
                 // both are virtual without identity: check contents
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/BoxNode.java	Wed Apr 10 02:10:14 2013 +0200
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/BoxNode.java	Wed Apr 10 10:17:35 2013 +0200
@@ -28,15 +28,15 @@
 import com.oracle.graal.nodes.type.*;
 import com.oracle.graal.nodes.virtual.*;
 
+/**
+ * This node represents the boxing of a primitive value. This corresponds to a call to the valueOf
+ * methods in Integer, Long, etc.
+ */
 public class BoxNode extends FixedWithNextNode implements VirtualizableAllocation, Lowerable, Canonicalizable {
 
     @Input private ValueNode value;
     private final Kind boxingKind;
 
-    public BoxNode(Invoke invoke) {
-        this(invoke.methodCallTarget().arguments().get(0), invoke.node().objectStamp().type(), invoke.methodCallTarget().targetMethod().getSignature().getParameterKind(0));
-    }
-
     public BoxNode(ValueNode value, ResolvedJavaType resultType, Kind boxingKind) {
         super(StampFactory.exactNonNull(resultType));
         this.value = value;
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/UnboxNode.java	Wed Apr 10 02:10:14 2013 +0200
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/UnboxNode.java	Wed Apr 10 10:17:35 2013 +0200
@@ -95,12 +95,6 @@
         return this;
     }
 
-    /*
-     * Normally, all these variants wouldn't be needed because this can be accomplished by using a
-     * generic method with automatic unboxing. These intrinsics, however, are themselves used for
-     * recognizing boxings, which means that there would be a circularity issue.
-     */
-
     @NodeIntrinsic
     public static native boolean unbox(Boolean value, @ConstantNodeParameter Kind kind);
 
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/spi/MacroSubstitution.java	Wed Apr 10 02:10:14 2013 +0200
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/spi/MacroSubstitution.java	Wed Apr 10 10:17:35 2013 +0200
@@ -77,6 +77,9 @@
     /**
      * Determines if this method should be substituted in all cases, even if inlining thinks it is
      * not important.
+     * 
+     * Not that this is still depending on whether inlining sees the correct call target, so it's
+     * only a hard guarantee for static and special invocations.
      */
-    boolean isForcedInlining() default false;
+    boolean forced() default false;
 }
--- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java	Wed Apr 10 02:10:14 2013 +0200
+++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java	Wed Apr 10 10:17:35 2013 +0200
@@ -149,7 +149,7 @@
                 Member originalMethod = originalMethod(classSubstitution, methodSubstitution.optional(), originalName, originalParameters);
                 if (originalMethod != null) {
                     ResolvedJavaMethod original = registerMethodSubstitution(originalMethod, substituteMethod);
-                    if (original != null && methodSubstitution.isForcedInlining()) {
+                    if (original != null && methodSubstitution.forced()) {
                         forcedSubstitutions.add(original);
                     }
                 }
@@ -160,7 +160,7 @@
                 Member originalMethod = originalMethod(classSubstitution, macroSubstitution.optional(), originalName, originalParameters);
                 if (originalMethod != null) {
                     ResolvedJavaMethod original = registerMacroSubstitution(originalMethod, macroSubstitution.macro());
-                    if (original != null && macroSubstitution.isForcedInlining()) {
+                    if (original != null && macroSubstitution.forced()) {
                         forcedSubstitutions.add(original);
                     }
                 }
--- a/graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/PartialEscapeClosure.java	Wed Apr 10 02:10:14 2013 +0200
+++ b/graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/PartialEscapeClosure.java	Wed Apr 10 10:17:35 2013 +0200
@@ -24,7 +24,6 @@
 
 import static com.oracle.graal.virtual.phases.ea.PartialEscapeAnalysisPhase.*;
 
-import java.io.*;
 import java.util.*;
 
 import com.oracle.graal.api.code.*;
@@ -47,7 +46,7 @@
 import com.oracle.graal.phases.schedule.*;
 import com.oracle.graal.virtual.nodes.*;
 import com.oracle.graal.virtual.phases.ea.BlockState.ReadCacheEntry;
-import com.oracle.graal.virtual.phases.ea.EffectList.*;
+import com.oracle.graal.virtual.phases.ea.EffectList.Effect;
 
 class PartialEscapeClosure extends BlockIteratorClosure<BlockState> {
 
@@ -90,21 +89,17 @@
         return changed;
     }
 
-    private static void trace2(String format, Object... args) {
-        Debug.log(format, args);
-    }
-
     public List<Node> applyEffects(final StructuredGraph graph) {
         final ArrayList<Node> obsoleteNodes = new ArrayList<>();
         BlockIteratorClosure<Object> closure = new BlockIteratorClosure<Object>() {
 
             private void apply(GraphEffectList effects, Object context) {
                 if (!effects.isEmpty()) {
-                    trace2(" ==== effects for %s", context);
+                    Debug.log(" ==== effects for %s", context);
                     for (Effect effect : effects) {
                         effect.apply(graph, obsoleteNodes);
                         if (effect.isVisible()) {
-                            trace2("    %s", effect);
+                            Debug.log("    %s", effect);
                         }
                     }
                 }
@@ -331,8 +326,6 @@
 
     }
 
-    static PrintStream out = System.out;
-
     @Override
     protected BlockState cloneState(BlockState oldState) {
         return oldState.cloneState();
@@ -394,7 +387,7 @@
                     ValueNode value = obj.getEntry(i);
                     ObjectState valueObj = exitState.getObjectState(value);
                     if (valueObj == null) {
-                        if ((value instanceof PhiNode && ((PhiNode) value).merge() == exitNode.loopBegin()) || initialObj == null || !initialObj.isVirtual() || initialObj.getEntry(i) != value) {
+                        if (exitNode.loopBegin().isPhiAtMerge(value) || initialObj == null || !initialObj.isVirtual() || initialObj.getEntry(i) != value) {
                             ProxyNode proxy = new ProxyNode(value, exitNode, PhiType.Value, null);
                             obj.setEntry(i, proxy);
                             effects.addFloatingNode(proxy, "virtualProxy");
@@ -413,11 +406,9 @@
                     }
                     obj.updateMaterializedValue(proxy);
                 } else {
-                    // assert initialObj.getMaterializedValue() == obj.getMaterializedValue() :
-                    // "materialized value is not allowed to change within loops: " +
-// initialObj.getMaterializedValue()
-                    // +
-                    // " vs. " + obj.getMaterializedValue() + " at " + exitNode;
+                    if (initialObj.getMaterializedValue() == obj.getMaterializedValue()) {
+                        Debug.log("materialized value changes within loop: %s vs. %s at %s", initialObj.getMaterializedValue(), obj.getMaterializedValue(), exitNode);
+                    }
                 }
             }
         }