# HG changeset patch # User Lukas Stadler # Date 1365581855 -7200 # Node ID 09cdf72247948a032d7f9dc186a21cdde2cee6ef # Parent 92b00825c037c541e4807322cbcf18db47473f86 PEA: cleanup and documentation for recent changes diff -r 92b00825c037 -r 09cdf7224794 graal/com.oracle.graal.api.replacements/src/com/oracle/graal/api/replacements/MethodSubstitution.java --- 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 diff -r 92b00825c037 -r 09cdf7224794 graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/BoxingSubstitutions.java --- 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); } diff -r 92b00825c037 -r 09cdf7224794 graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/calc/ObjectEqualsNode.java --- 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 diff -r 92b00825c037 -r 09cdf7224794 graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/BoxNode.java --- 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; diff -r 92b00825c037 -r 09cdf7224794 graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/UnboxNode.java --- 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); diff -r 92b00825c037 -r 09cdf7224794 graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/spi/MacroSubstitution.java --- 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; } diff -r 92b00825c037 -r 09cdf7224794 graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java --- 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); } } diff -r 92b00825c037 -r 09cdf7224794 graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/PartialEscapeClosure.java --- 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 { @@ -90,21 +89,17 @@ return changed; } - private static void trace2(String format, Object... args) { - Debug.log(format, args); - } - public List applyEffects(final StructuredGraph graph) { final ArrayList obsoleteNodes = new ArrayList<>(); BlockIteratorClosure closure = new BlockIteratorClosure() { 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); + } } } }