# HG changeset patch # User Stefan Anzinger # Date 1417443599 -3600 # Node ID 51d05f25830935d1065164c129381ffd59895fae # Parent 68814fb4bbe4c6becb609d843267521622e05f87# Parent 12bd2b344b088225fe4116f017dbdeae201b74f5 Merge diff -r 68814fb4bbe4 -r 51d05f258309 graal/com.oracle.graal.api.meta/src/com/oracle/graal/api/meta/AbstractJavaProfile.java --- a/graal/com.oracle.graal.api.meta/src/com/oracle/graal/api/meta/AbstractJavaProfile.java Mon Dec 01 15:19:28 2014 +0100 +++ b/graal/com.oracle.graal.api.meta/src/com/oracle/graal/api/meta/AbstractJavaProfile.java Mon Dec 01 15:19:59 2014 +0100 @@ -84,7 +84,7 @@ public T findEntry(ResolvedJavaType type) { if (pitems != null) { for (T pt : pitems) { - if (pt.getItem() == type) { + if (pt.getItem().equals(type)) { return pt; } } diff -r 68814fb4bbe4 -r 51d05f258309 graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotResolvedJavaFieldImpl.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotResolvedJavaFieldImpl.java Mon Dec 01 15:19:28 2014 +0100 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotResolvedJavaFieldImpl.java Mon Dec 01 15:19:59 2014 +0100 @@ -211,7 +211,7 @@ * @return {@code true} if the field is a well-known stable field. */ public static boolean test(HotSpotResolvedJavaField field) { - return field == STRING_VALUE_FIELD; + return field.equals(STRING_VALUE_FIELD); } private static final ResolvedJavaField STRING_VALUE_FIELD; diff -r 68814fb4bbe4 -r 51d05f258309 graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/AddLocationNode.java --- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/AddLocationNode.java Mon Dec 01 15:19:28 2014 +0100 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/AddLocationNode.java Mon Dec 01 15:19:59 2014 +0100 @@ -51,7 +51,7 @@ } public static AddLocationNode create(LocationNode x, LocationNode y, Graph graph) { - assert x.getValueKind().equals(y.getValueKind()) && x.getLocationIdentity() == y.getLocationIdentity(); + assert x.getValueKind().equals(y.getValueKind()) && x.getLocationIdentity().equals(y.getLocationIdentity()); return graph.unique(AddLocationNode.create(x, y)); } diff -r 68814fb4bbe4 -r 51d05f258309 graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/FloatableAccessNode.java --- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/FloatableAccessNode.java Mon Dec 01 15:19:28 2014 +0100 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/FloatableAccessNode.java Mon Dec 01 15:19:59 2014 +0100 @@ -53,6 +53,6 @@ * an attached write barrier with pre-semantics can not also float. */ public boolean canFloat() { - return location().getLocationIdentity() != LocationIdentity.ANY_LOCATION && getBarrierType() == BarrierType.NONE; + return !location().getLocationIdentity().equals(LocationIdentity.ANY_LOCATION) && getBarrierType() == BarrierType.NONE; } } diff -r 68814fb4bbe4 -r 51d05f258309 graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/ReadNode.java --- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/ReadNode.java Mon Dec 01 15:19:28 2014 +0100 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/ReadNode.java Mon Dec 01 15:19:59 2014 +0100 @@ -128,7 +128,7 @@ } } } - if (location.getLocationIdentity() == LocationIdentity.ARRAY_LENGTH_LOCATION) { + if (location.getLocationIdentity().equals(LocationIdentity.ARRAY_LENGTH_LOCATION)) { ValueNode length = GraphUtil.arrayLength(object); if (length != null) { // TODO Does this need a PiCastNode to the positive range? diff -r 68814fb4bbe4 -r 51d05f258309 graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/UnsafeAccessNode.java --- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/UnsafeAccessNode.java Mon Dec 01 15:19:28 2014 +0100 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/UnsafeAccessNode.java Mon Dec 01 15:19:59 2014 +0100 @@ -65,7 +65,7 @@ @Override public Node canonical(CanonicalizerTool tool) { - if (this.getLocationIdentity() == LocationIdentity.ANY_LOCATION && offset().isConstant()) { + if (this.getLocationIdentity().equals(LocationIdentity.ANY_LOCATION) && offset().isConstant()) { long constantOffset = offset().asJavaConstant().asLong(); // Try to canonicalize to a field access. @@ -81,7 +81,7 @@ } } } - if (this.getLocationIdentity() == LocationIdentity.ANY_LOCATION) { + if (this.getLocationIdentity().equals(LocationIdentity.ANY_LOCATION)) { ResolvedJavaType receiverType = StampTool.typeOrNull(object()); // Try to build a better location identity. if (receiverType != null && receiverType.isArray()) { diff -r 68814fb4bbe4 -r 51d05f258309 graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/ConditionalEliminationPhase.java --- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/ConditionalEliminationPhase.java Mon Dec 01 15:19:28 2014 +0100 +++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/ConditionalEliminationPhase.java Mon Dec 01 15:19:59 2014 +0100 @@ -148,7 +148,7 @@ break; } } - if (type != null && type != StampTool.typeOrNull(node)) { + if (type != null && !type.equals(StampTool.typeOrNull(node))) { newKnownTypes.put(node, type); } } diff -r 68814fb4bbe4 -r 51d05f258309 graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/FloatingReadPhase.java --- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/FloatingReadPhase.java Mon Dec 01 15:19:28 2014 +0100 +++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/FloatingReadPhase.java Mon Dec 01 15:19:59 2014 +0100 @@ -304,7 +304,7 @@ private static void processAccess(MemoryAccess access, MemoryMapImpl state) { LocationIdentity locationIdentity = access.getLocationIdentity(); - if (locationIdentity != LocationIdentity.ANY_LOCATION) { + if (!locationIdentity.equals(LocationIdentity.ANY_LOCATION)) { MemoryNode lastLocationAccess = state.getLastLocationAccess(locationIdentity); access.setLastLocationAccess(lastLocationAccess); } @@ -321,7 +321,7 @@ } private static void processIdentity(LocationIdentity identity, MemoryCheckpoint checkpoint, MemoryMapImpl state) { - if (identity == ANY_LOCATION) { + if (identity.equals(ANY_LOCATION)) { state.lastMemorySnapshot.clear(); } state.lastMemorySnapshot.put(identity, checkpoint); diff -r 68814fb4bbe4 -r 51d05f258309 graal/com.oracle.graal.phases/src/com/oracle/graal/phases/schedule/SchedulePhase.java --- a/graal/com.oracle.graal.phases/src/com/oracle/graal/phases/schedule/SchedulePhase.java Mon Dec 01 15:19:28 2014 +0100 +++ b/graal/com.oracle.graal.phases/src/com/oracle/graal/phases/schedule/SchedulePhase.java Mon Dec 01 15:19:59 2014 +0100 @@ -1045,9 +1045,9 @@ LocationIdentity readLocation = frn.location().getLocationIdentity(); assert !readLocation.isImmutable(); if (frn.getLastLocationAccess() == node) { - assert identity == ANY_LOCATION || readLocation == identity || node instanceof MemoryCheckpoint.Multi : "location doesn't match: " + readLocation + ", " + identity; + assert identity.equals(ANY_LOCATION) || readLocation.equals(identity) || node instanceof MemoryCheckpoint.Multi : "location doesn't match: " + readLocation + ", " + identity; state.clearBeforeLastLocation(frn); - } else if (!state.isBeforeLastLocation(frn) && (readLocation == identity || (node != getCFG().graph.start() && ANY_LOCATION == identity))) { + } else if (!state.isBeforeLastLocation(frn) && (readLocation.equals(identity) || (node != getCFG().graph.start() && ANY_LOCATION.equals(identity)))) { state.removeRead(frn); addToLatestSorting(frn, state); } diff -r 68814fb4bbe4 -r 51d05f258309 graal/com.oracle.graal.phases/src/com/oracle/graal/phases/verify/VerifyUsageWithEquals.java --- a/graal/com.oracle.graal.phases/src/com/oracle/graal/phases/verify/VerifyUsageWithEquals.java Mon Dec 01 15:19:28 2014 +0100 +++ b/graal/com.oracle.graal.phases/src/com/oracle/graal/phases/verify/VerifyUsageWithEquals.java Mon Dec 01 15:19:59 2014 +0100 @@ -31,24 +31,30 @@ import com.oracle.graal.phases.tiers.*; /** - * For certain types object identity should not be used for object equality check. This phase checks - * the correct usage of the given type. Equality checks with == or != (except null checks) results - * in an {@link AssertionError}. + * For certain types, object identity should not be used for object equality check. This phase + * checks the correct usage of the given type. Equality checks with == or != (except null checks) + * results in an {@link AssertionError}. */ public class VerifyUsageWithEquals extends VerifyPhase { - private final Class klass; + /** + * The type of values that must not use identity for testing object equality. + */ + private final Class restrictedClass; public VerifyUsageWithEquals(Class klass) { - this.klass = klass; + this.restrictedClass = klass; } - private boolean isAssignableType(ValueNode node, MetaAccessProvider metaAccess) { + /** + * Determines whether the type of {@code node} is assignable to the {@link #restrictedClass}. + */ + private boolean isAssignableToRestrictedType(ValueNode node, MetaAccessProvider metaAccess) { if (node.stamp() instanceof ObjectStamp) { - ResolvedJavaType valueType = metaAccess.lookupJavaType(klass); + ResolvedJavaType restrictedType = metaAccess.lookupJavaType(restrictedClass); ResolvedJavaType nodeType = StampTool.typeOrNull(node); - if (nodeType != null && valueType.isAssignableFrom(nodeType)) { + if (nodeType != null && restrictedType.isAssignableFrom(nodeType)) { return true; } } @@ -59,16 +65,47 @@ return node.isConstant() && node.isNullConstant(); } - private boolean checkUsage(ValueNode x, ValueNode y, MetaAccessProvider metaAccess) { - return isAssignableType(x, metaAccess) && !isNullConstant(y); + private static boolean isEqualsMethod(ResolvedJavaMethod method) { + if (method.getName().equals("equals")) { + Signature sig = method.getSignature(); + if (sig.getReturnKind() == Kind.Boolean) { + if (sig.getParameterCount(false) == 1) { + ResolvedJavaType ptype = (ResolvedJavaType) sig.getParameterType(0, method.getDeclaringClass()); + if (ptype.isJavaLangObject()) { + return true; + } + + } + } + } + return false; + } + + private static boolean isThisParameter(ValueNode node) { + return node instanceof ParameterNode && ((ParameterNode) node).index() == 0; + } + + /** + * Checks whether the type of {@code x} is assignable to the restricted type and that {@code y} + * is not a null constant. + */ + private boolean isIllegalUsage(ResolvedJavaMethod method, ValueNode x, ValueNode y, MetaAccessProvider metaAccess) { + if (isAssignableToRestrictedType(x, metaAccess) && !isNullConstant(y)) { + if (isEqualsMethod(method) && isThisParameter(x) || isThisParameter(y)) { + return false; + } + return true; + } + return false; } @Override protected boolean verify(StructuredGraph graph, PhaseContext context) { for (ObjectEqualsNode cn : graph.getNodes().filter(ObjectEqualsNode.class)) { // bail out if we compare an object of type klass with == or != (except null checks) - if (checkUsage(cn.getX(), cn.getY(), context.getMetaAccess()) && checkUsage(cn.getY(), cn.getX(), context.getMetaAccess())) { - throw new VerificationError("Verification of " + klass.getName() + " usage failed: Comparing " + cn.getX() + " and " + cn.getY() + " in " + graph.method() + + ResolvedJavaMethod method = graph.method(); + if (isIllegalUsage(method, cn.getX(), cn.getY(), context.getMetaAccess()) || isIllegalUsage(method, cn.getY(), cn.getX(), context.getMetaAccess())) { + throw new VerificationError("Verification of " + restrictedClass.getName() + " usage failed: Comparing " + cn.getX() + " and " + cn.getY() + " in " + method + " must use .equals() for object equality, not '==' or '!='"); } } diff -r 68814fb4bbe4 -r 51d05f258309 graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetTemplate.java --- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetTemplate.java Mon Dec 01 15:19:28 2014 +0100 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetTemplate.java Mon Dec 01 15:19:59 2014 +0100 @@ -1016,7 +1016,7 @@ if (replacee instanceof MemoryCheckpoint.Single) { // check if some node in snippet graph also kills the same location LocationIdentity locationIdentity = ((MemoryCheckpoint.Single) replacee).getLocationIdentity(); - if (locationIdentity == ANY_LOCATION) { + if (locationIdentity.equals(ANY_LOCATION)) { assert !(memoryMap.getLastLocationAccess(ANY_LOCATION) instanceof MemoryAnchorNode) : replacee + " kills ANY_LOCATION, but snippet does not"; } assert kills.contains(locationIdentity) : replacee + " kills " + locationIdentity + ", but snippet doesn't contain a kill to this location"; @@ -1046,7 +1046,7 @@ if (SnippetCounters.getValue()) { // accesses to snippet counters are artificially introduced and violate the memory // semantics. - if (locationIdentity == SnippetCounter.SNIPPET_COUNTER_LOCATION) { + if (locationIdentity.equals(SnippetCounter.SNIPPET_COUNTER_LOCATION)) { continue; } } diff -r 68814fb4bbe4 -r 51d05f258309 graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/PartialEvaluator.java --- a/graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/PartialEvaluator.java Mon Dec 01 15:19:28 2014 +0100 +++ b/graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/PartialEvaluator.java Mon Dec 01 15:19:59 2014 +0100 @@ -416,7 +416,7 @@ } FrameState directCallState = invoke.stateAfter(); - while (directCallState != null && directCallState.method() != callSiteProxyMethod) { + while (directCallState != null && !directCallState.method().equals(callSiteProxyMethod)) { directCallState = directCallState.outerFrameState(); } diff -r 68814fb4bbe4 -r 51d05f258309 graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/PEReadEliminationClosure.java --- a/graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/PEReadEliminationClosure.java Mon Dec 01 15:19:28 2014 +0100 +++ b/graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/PEReadEliminationClosure.java Mon Dec 01 15:19:59 2014 +0100 @@ -111,7 +111,7 @@ private static void processIdentity(PEReadEliminationBlockState state, LocationIdentity identity) { if (identity instanceof ResolvedJavaField) { state.killReadCache((ResolvedJavaField) identity); - } else if (identity == ANY_LOCATION) { + } else if (identity.equals(ANY_LOCATION)) { state.killReadCache(); } } diff -r 68814fb4bbe4 -r 51d05f258309 graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/ReadEliminationBlockState.java --- a/graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/ReadEliminationBlockState.java Mon Dec 01 15:19:28 2014 +0100 +++ b/graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/ReadEliminationBlockState.java Mon Dec 01 15:19:59 2014 +0100 @@ -80,7 +80,7 @@ @Override public boolean conflicts(LocationIdentity other) { - return identity == other; + return identity.equals(other); } } @@ -106,7 +106,7 @@ @Override public boolean conflicts(LocationIdentity other) { - return locationIdentity == other; + return locationIdentity.equals(other); } } @@ -123,7 +123,7 @@ @Override public boolean conflicts(LocationIdentity other) { - return identity.getLocationIdentity() == other; + return identity.getLocationIdentity().equals(other); } } diff -r 68814fb4bbe4 -r 51d05f258309 graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/ReadEliminationClosure.java --- a/graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/ReadEliminationClosure.java Mon Dec 01 15:19:28 2014 +0100 +++ b/graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/ReadEliminationClosure.java Mon Dec 01 15:19:59 2014 +0100 @@ -118,7 +118,7 @@ } else if (node instanceof UnsafeAccessNode) { if (node instanceof UnsafeLoadNode) { UnsafeLoadNode load = (UnsafeLoadNode) node; - if (load.offset().isConstant() && load.getLocationIdentity() != LocationIdentity.ANY_LOCATION) { + if (load.offset().isConstant() && !load.getLocationIdentity().equals(LocationIdentity.ANY_LOCATION)) { ValueNode object = GraphUtil.unproxify(load.object()); UnsafeLoadCacheEntry identifier = new UnsafeLoadCacheEntry(object, load.offset(), load.getLocationIdentity()); ValueNode cachedValue = state.getCacheEntry(identifier); @@ -133,7 +133,7 @@ } else { assert node instanceof UnsafeStoreNode; UnsafeStoreNode write = (UnsafeStoreNode) node; - if (write.offset().isConstant() && write.getLocationIdentity() != LocationIdentity.ANY_LOCATION) { + if (write.offset().isConstant() && !write.getLocationIdentity().equals(LocationIdentity.ANY_LOCATION)) { ValueNode object = GraphUtil.unproxify(write.object()); UnsafeLoadCacheEntry identifier = new UnsafeLoadCacheEntry(object, write.offset(), write.getLocationIdentity()); ValueNode cachedValue = state.getCacheEntry(identifier); @@ -161,7 +161,7 @@ } private static void processIdentity(ReadEliminationBlockState state, LocationIdentity identity) { - if (identity == ANY_LOCATION) { + if (identity.equals(ANY_LOCATION)) { state.killReadCache(); return; }