Mercurial > hg > truffle
changeset 14976:82399ac30721
make implicit null checking optional for Access nodes (fixes CAS crash)
line wrap: on
line diff
--- a/graal/com.oracle.graal.compiler.amd64/src/com/oracle/graal/compiler/amd64/AMD64MemoryPeephole.java Fri Apr 04 16:59:01 2014 +0200 +++ b/graal/com.oracle.graal.compiler.amd64/src/com/oracle/graal/compiler/amd64/AMD64MemoryPeephole.java Fri Apr 04 17:06:44 2014 +0200 @@ -77,7 +77,7 @@ } protected AMD64AddressValue makeAddress(Access access) { - return (AMD64AddressValue) access.nullCheckLocation().generateAddress(gen, gen.operand(access.object())); + return (AMD64AddressValue) access.accessLocation().generateAddress(gen, gen.operand(access.object())); } protected Value emitBinaryMemory(AMD64Arithmetic op, boolean commutative, ValueNode x, ValueNode y, Access access) { @@ -90,7 +90,7 @@ } } ensureEvaluated(other); - return gen.getLIRGenerator().emitBinaryMemory(op, access.nullCheckLocation().getValueKind(), gen.getLIRGeneratorTool().asAllocatable(gen.operand(other)), makeAddress(access), getState(access)); + return gen.getLIRGenerator().emitBinaryMemory(op, access.accessLocation().getValueKind(), gen.getLIRGeneratorTool().asAllocatable(gen.operand(other)), makeAddress(access), getState(access)); } /** @@ -129,7 +129,7 @@ @Override public Value emitAddMemory(ValueNode x, ValueNode y, Access access) { - switch (access.nullCheckLocation().getValueKind()) { + switch (access.accessLocation().getValueKind()) { case Int: return emitBinaryMemory(IADD, true, x, y, access); case Long: @@ -145,7 +145,7 @@ @Override public Value emitSubMemory(ValueNode x, ValueNode y, Access access) { - switch (access.nullCheckLocation().getValueKind()) { + switch (access.accessLocation().getValueKind()) { case Int: return emitBinaryMemory(ISUB, false, x, y, access); case Long: @@ -161,7 +161,7 @@ @Override public Value emitMulMemory(ValueNode x, ValueNode y, Access access) { - switch (access.nullCheckLocation().getValueKind()) { + switch (access.accessLocation().getValueKind()) { case Int: return emitBinaryMemory(IMUL, true, x, y, access); case Long: @@ -187,7 +187,7 @@ @Override public Value emitAndMemory(ValueNode x, ValueNode y, Access access) { - Kind kind = access.nullCheckLocation().getValueKind(); + Kind kind = access.accessLocation().getValueKind(); switch (kind) { case Int: return emitBinaryMemory(IAND, true, x, y, access); @@ -224,7 +224,7 @@ @Override public Value emitOrMemory(ValueNode x, ValueNode y, Access access) { - switch (access.nullCheckLocation().getValueKind()) { + switch (access.accessLocation().getValueKind()) { case Int: return emitBinaryMemory(IOR, true, x, y, access); case Long: @@ -236,7 +236,7 @@ @Override public Value emitXorMemory(ValueNode x, ValueNode y, Access access) { - switch (access.nullCheckLocation().getValueKind()) { + switch (access.accessLocation().getValueKind()) { case Int: return emitBinaryMemory(IXOR, true, x, y, access); case Long: @@ -249,7 +249,7 @@ @Override public Value emitReinterpretMemory(Stamp stamp, Access access) { PlatformKind to = gen.getLIRGenerator().getPlatformKind(stamp); - Kind from = access.nullCheckLocation().getValueKind(); + Kind from = access.accessLocation().getValueKind(); assert to != from : "should have been eliminated"; /* @@ -355,7 +355,7 @@ @Override public Value emitZeroExtendMemory(int inputBits, int resultBits, Access access) { assert resultBits == 32 || resultBits == 64; - Kind memoryKind = access.nullCheckLocation().getValueKind(); + Kind memoryKind = access.accessLocation().getValueKind(); if (memoryKind.getBitCount() != inputBits && !memoryKind.isUnsigned()) { // The memory being read from is signed and smaller than the result size so // this is a sign extension to inputBits followed by a zero extension to resultBits @@ -399,7 +399,7 @@ private boolean emitIntegerTestBranchMemory(ValueNode left, ValueNode right, Access access, LabelRef trueLabel, LabelRef falseLabel, double trueLabelProbability) { ValueNode other = selectOtherInput(left, right, access); - Kind kind = access.nullCheckLocation().getValueKind(); + Kind kind = access.accessLocation().getValueKind(); if (other.isConstant()) { if (kind != kind.getStackKind()) { return false; @@ -439,7 +439,7 @@ protected boolean emitCompareBranchMemory(ValueNode left, ValueNode right, Access access, Condition cond, boolean unorderedIsTrue, LabelRef trueLabel, LabelRef falseLabel, double trueLabelProbability) { ValueNode other = selectOtherInput(left, right, access); - Kind kind = access.nullCheckLocation().getValueKind(); + Kind kind = access.accessLocation().getValueKind(); boolean mirrored = false; if (other.isConstant()) {
--- a/graal/com.oracle.graal.hotspot.amd64/src/com/oracle/graal/hotspot/amd64/AMD64HotSpotMemoryPeephole.java Fri Apr 04 16:59:01 2014 +0200 +++ b/graal/com.oracle.graal.hotspot.amd64/src/com/oracle/graal/hotspot/amd64/AMD64HotSpotMemoryPeephole.java Fri Apr 04 17:06:44 2014 +0200 @@ -86,7 +86,7 @@ double trueLabelProbability) { if (HotSpotGraalRuntime.runtime().getConfig().useCompressedOops) { ValueNode other = selectOtherInput(left, right, access); - Kind kind = access.nullCheckLocation().getValueKind(); + Kind kind = access.accessLocation().getValueKind(); if (other.isConstant() && kind == Kind.Object && access.isCompressible()) { ensureEvaluated(other);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/graal/com.oracle.graal.jtt/src/com/oracle/graal/jtt/jdk/Unsafe_compareAndSwapNullCheck.java Fri Apr 04 17:06:44 2014 +0200 @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2008, 2014, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package com.oracle.graal.jtt.jdk; + +import org.junit.*; + +import sun.misc.*; + +import com.oracle.graal.jtt.*; + +public class Unsafe_compareAndSwapNullCheck extends JTTTest { + + static final Unsafe unsafe = UnsafeAccess01.getUnsafe(); + static final long valueOffset; + static { + try { + valueOffset = unsafe.objectFieldOffset(Unsafe_compareAndSwap.class.getDeclaredField("value")); + } catch (Exception ex) { + throw new Error(ex); + } + } + + long value; + long lng; + + public static void test(Unsafe_compareAndSwapNullCheck u, long expected, long newValue) { + @SuppressWarnings("unused") + long l = u.lng; + unsafe.compareAndSwapLong(u, valueOffset, expected, newValue); + } + + @Test + public void run0() throws Throwable { + runTest(EMPTY, false, true, "test", null, 1L, 2L); + } +}
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/calc/FloatConvertNode.java Fri Apr 04 16:59:01 2014 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/calc/FloatConvertNode.java Fri Apr 04 17:06:44 2014 +0200 @@ -206,7 +206,7 @@ } public boolean generate(MemoryArithmeticLIRLowerer gen, Access access) { - Kind kind = access.nullCheckLocation().getValueKind(); + Kind kind = access.accessLocation().getValueKind(); if (kind != kind.getStackKind()) { // Doesn't work for subword operations return false;
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/calc/SignExtendNode.java Fri Apr 04 16:59:01 2014 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/calc/SignExtendNode.java Fri Apr 04 17:06:44 2014 +0200 @@ -111,7 +111,7 @@ @Override public boolean generate(MemoryArithmeticLIRLowerer gen, Access access) { - Value result = gen.emitSignExtendMemory(access, access.nullCheckLocation().getValueKind().getBitCount(), getResultBits()); + Value result = gen.emitSignExtendMemory(access, access.accessLocation().getValueKind().getBitCount(), getResultBits()); if (result != null) { gen.setResult(this, result); }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/Access.java Fri Apr 04 16:59:01 2014 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/Access.java Fri Apr 04 17:06:44 2014 +0200 @@ -28,6 +28,8 @@ ValueNode object(); - LocationNode nullCheckLocation(); + LocationNode accessLocation(); + + boolean canNullCheck(); }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/FixedAccessNode.java Fri Apr 04 16:59:01 2014 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/FixedAccessNode.java Fri Apr 04 17:06:44 2014 +0200 @@ -27,8 +27,7 @@ /** * Accesses a value at an memory address specified by an {@linkplain #object object} and a - * {@linkplain #nullCheckLocation() location}. The access does not include a null check on the - * object. + * {@linkplain #accessLocation() location}. The access does not include a null check on the object. */ public abstract class FixedAccessNode extends DeoptimizingFixedWithNextNode implements Access, GuardingNode { @@ -52,7 +51,7 @@ return (LocationNode) location; } - public LocationNode nullCheckLocation() { + public LocationNode accessLocation() { return (LocationNode) location; }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/FloatingAccessNode.java Fri Apr 04 16:59:01 2014 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/FloatingAccessNode.java Fri Apr 04 17:06:44 2014 +0200 @@ -41,7 +41,7 @@ return location; } - public LocationNode nullCheckLocation() { + public LocationNode accessLocation() { return location; } @@ -73,5 +73,9 @@ return compressible; } + public boolean canNullCheck() { + return true; + } + public abstract FixedAccessNode asFixedNode(); }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/FloatingReadNode.java Fri Apr 04 16:59:01 2014 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/FloatingReadNode.java Fri Apr 04 17:06:44 2014 +0200 @@ -76,7 +76,7 @@ @Override public FixedAccessNode asFixedNode() { - return graph().add(new ReadNode(object(), nullCheckLocation(), stamp(), getGuard(), getBarrierType(), isCompressible())); + return graph().add(new ReadNode(object(), accessLocation(), stamp(), getGuard(), getBarrierType(), isCompressible())); } @Override
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/JavaReadNode.java Fri Apr 04 16:59:01 2014 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/JavaReadNode.java Fri Apr 04 17:06:44 2014 +0200 @@ -39,4 +39,8 @@ public void lower(LoweringTool tool) { tool.getLowerer().lower(this, tool); } + + public boolean canNullCheck() { + return true; + } }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/JavaWriteNode.java Fri Apr 04 16:59:01 2014 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/JavaWriteNode.java Fri Apr 04 17:06:44 2014 +0200 @@ -38,4 +38,8 @@ public void lower(LoweringTool tool) { tool.getLowerer().lower(this, tool); } + + public boolean canNullCheck() { + return true; + } }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/ReadNode.java Fri Apr 04 16:59:01 2014 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/ReadNode.java Fri Apr 04 17:06:44 2014 +0200 @@ -164,4 +164,8 @@ } } } + + public boolean canNullCheck() { + return true; + } }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/WriteNode.java Fri Apr 04 16:59:01 2014 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/WriteNode.java Fri Apr 04 17:06:44 2014 +0200 @@ -82,4 +82,8 @@ } } } + + public boolean canNullCheck() { + return true; + } }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/java/LoweredCompareAndSwapNode.java Fri Apr 04 16:59:01 2014 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/java/LoweredCompareAndSwapNode.java Fri Apr 04 17:06:44 2014 +0200 @@ -71,6 +71,10 @@ return location().getLocationIdentity(); } + public boolean canNullCheck() { + return false; + } + @Override public void generate(NodeLIRBuilderTool gen) { gen.visitCompareAndSwap(this, location().generateAddress(gen, gen.operand(object())));
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/GuardLoweringPhase.java Fri Apr 04 16:59:01 2014 +0200 +++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/GuardLoweringPhase.java Fri Apr 04 17:06:44 2014 +0200 @@ -45,7 +45,7 @@ /** * This phase lowers {@link GuardNode GuardNodes} into corresponding control-flow structure and * {@link DeoptimizeNode DeoptimizeNodes}. - * + * * This allow to enter the {@link GuardsStage#FIXED_DEOPTS FIXED_DEOPTS} stage of the graph where * all node that may cause deoptimization are fixed. * <p> @@ -89,24 +89,26 @@ } private void processAccess(Access access) { - GuardNode guard = nullGuarded.get(access.object()); - if (guard != null && isImplicitNullCheck(access.nullCheckLocation())) { - metricImplicitNullCheck.increment(); - access.setGuard(guard.getGuard()); - FixedAccessNode fixedAccess; - if (access instanceof FloatingAccessNode) { - fixedAccess = ((FloatingAccessNode) access).asFixedNode(); - replaceCurrent(fixedAccess.asNode()); - } else { - fixedAccess = (FixedAccessNode) access; + if (access.canNullCheck()) { + GuardNode guard = nullGuarded.get(access.object()); + if (guard != null && isImplicitNullCheck(access.accessLocation())) { + metricImplicitNullCheck.increment(); + access.setGuard(guard.getGuard()); + FixedAccessNode fixedAccess; + if (access instanceof FloatingAccessNode) { + fixedAccess = ((FloatingAccessNode) access).asFixedNode(); + replaceCurrent(fixedAccess); + } else { + fixedAccess = (FixedAccessNode) access; + } + fixedAccess.setNullCheck(true); + LogicNode condition = guard.condition(); + guard.replaceAndDelete(fixedAccess); + if (condition.usages().isEmpty()) { + GraphUtil.killWithUnusedFloatingInputs(condition); + } + nullGuarded.remove(fixedAccess.object()); } - fixedAccess.setNullCheck(true); - LogicNode condition = guard.condition(); - guard.replaceAndDelete(fixedAccess.asNode()); - if (condition.usages().isEmpty()) { - GraphUtil.killWithUnusedFloatingInputs(condition); - } - nullGuarded.remove(fixedAccess.object()); } }