# HG changeset patch # User Doug Simon # Date 1347980364 -7200 # Node ID b74402a7079baff1ed62350d079496aa15956fcf # Parent aa57aa781e867b6f2d9f8f9d2fc7da9c86c5c3b6 fixed oopmap bug caused by unsafe mixing of word and object values diff -r aa57aa781e86 -r b74402a7079b graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java --- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java Tue Sep 18 16:58:09 2012 +0200 +++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java Tue Sep 18 16:59:24 2012 +0200 @@ -89,6 +89,12 @@ * (and is assumed to be a {@link Node} subclass). */ Class value() default NodeIntrinsic.class; + + /** + * Determines if the stamp of the instantiated intrinsic node has its stamp set + * from the return type of the annotated method. + */ + boolean setStampFromReturnType() default false; } public interface ValueNumberable {} diff -r aa57aa781e86 -r b74402a7079b graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/nodes/RegisterNode.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/nodes/RegisterNode.java Tue Sep 18 16:58:09 2012 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/nodes/RegisterNode.java Tue Sep 18 16:59:24 2012 +0200 @@ -42,6 +42,11 @@ this.register = register; } + public RegisterNode(Register register) { + super(StampFactory.object()); + this.register = register; + } + @Override public void generate(LIRGeneratorTool generator) { Value result; @@ -64,10 +69,4 @@ return super.toString(verbosity); } } - - @SuppressWarnings("unused") - @NodeIntrinsic - public static T register(@ConstantNodeParameter Register register, @ConstantNodeParameter Kind kind) { - throw new UnsupportedOperationException(); - } } diff -r aa57aa781e86 -r b74402a7079b graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/HotSpotSnippetUtils.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/HotSpotSnippetUtils.java Tue Sep 18 16:58:09 2012 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/HotSpotSnippetUtils.java Tue Sep 18 16:59:24 2012 +0200 @@ -22,11 +22,11 @@ */ package com.oracle.graal.hotspot.snippets; -import static com.oracle.graal.nodes.extended.UnsafeLoadNode.*; - import com.oracle.graal.api.code.*; import com.oracle.graal.api.code.Register.RegisterFlag; import com.oracle.graal.api.meta.*; +import com.oracle.graal.graph.Node.ConstantNodeParameter; +import com.oracle.graal.graph.Node.NodeIntrinsic; import com.oracle.graal.hotspot.*; import com.oracle.graal.hotspot.nodes.*; import com.oracle.graal.nodes.extended.*; @@ -62,12 +62,12 @@ } @Fold - static Register threadReg() { + static Register threadRegister() { return HotSpotGraalRuntime.getInstance().getConfig().threadRegister; } @Fold - static Register stackPointerReg() { + static Register stackPointerRegister() { return AMD64.rsp; } @@ -204,13 +204,44 @@ return object; } - static Word asWord(Object object) { - return Word.fromObject(object); + /** + * Gets the value of the stack pointer register as a Word. + */ + static Word stackPointer() { + return HotSpotSnippetUtils.registerAsWord(stackPointerRegister()/*, wordKind()*/); + } + + /** + * Gets the value of the thread register as a Word. + */ + static Word thread() { + return HotSpotSnippetUtils.registerAsWord(threadRegister()/*, wordKind()*/); + } + + static Word loadWordFromWord(Word address, int offset) { + return loadWordFromWordIntrinsic(address, 0, offset, wordKind()); } - static Word loadWord(Word address, int offset) { - Object value = loadObject(address, 0, offset, true); - return asWord(value); + static Word loadWordFromObject(Object object, int offset) { + return loadWordFromObjectIntrinsic(object, 0, offset, wordKind()); + } + + @SuppressWarnings("unused") + @NodeIntrinsic(value = RegisterNode.class, setStampFromReturnType = true) + public static Word registerAsWord(@ConstantNodeParameter Register register) { + throw new UnsupportedOperationException(); + } + + @SuppressWarnings("unused") + @NodeIntrinsic(value = UnsafeLoadNode.class, setStampFromReturnType = true) + private static Word loadWordFromObjectIntrinsic(Object object, @ConstantNodeParameter int displacement, long offset, @ConstantNodeParameter Kind wordKind) { + throw new UnsupportedOperationException("This method may only be compiled with the Graal compiler"); + } + + @SuppressWarnings("unused") + @NodeIntrinsic(value = UnsafeLoadNode.class, setStampFromReturnType = true) + private static Word loadWordFromWordIntrinsic(Word address, @ConstantNodeParameter int displacement, long offset, @ConstantNodeParameter Kind wordKind) { + throw new UnsupportedOperationException("This method may only be compiled with the Graal compiler"); } static { diff -r aa57aa781e86 -r b74402a7079b graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/MonitorSnippets.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/MonitorSnippets.java Tue Sep 18 16:58:09 2012 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/MonitorSnippets.java Tue Sep 18 16:59:24 2012 +0200 @@ -25,9 +25,7 @@ import static com.oracle.graal.hotspot.nodes.BeginLockScopeNode.*; import static com.oracle.graal.hotspot.nodes.DirectCompareAndSwapNode.*; import static com.oracle.graal.hotspot.nodes.EndLockScopeNode.*; -import static com.oracle.graal.hotspot.nodes.RegisterNode.*; import static com.oracle.graal.hotspot.snippets.HotSpotSnippetUtils.*; -import static com.oracle.graal.nodes.extended.UnsafeLoadNode.*; import static com.oracle.graal.snippets.SnippetTemplate.Arguments.*; import static com.oracle.graal.snippets.nodes.DirectObjectStoreNode.*; @@ -74,7 +72,7 @@ verifyOop(object); // Load the mark word - this includes a null-check on object - final Word mark = asWord(loadObject(object, 0, markOffset(), false)); + final Word mark = loadWordFromObject(object, markOffset()); final Word lock = beginLockScope(object, false, wordKind()); @@ -92,8 +90,8 @@ // The bias pattern is present in the object's mark word. Need to check // whether the bias owner and the epoch are both still current. Object hub = loadHub(object); - final Word prototypeMarkWord = asWord(loadObject(hub, 0, prototypeMarkWordOffset(), true)); - final Word thread = asWord(register(threadReg(), wordKind())); + final Word prototypeMarkWord = loadWordFromObject(hub, prototypeMarkWordOffset()); + final Word thread = thread(); final Word tmp = prototypeMarkWord.or(thread).xor(mark).and(~ageMaskInPlace()); if (tmp == Word.zero()) { // Object is already biased to current thread -> done @@ -201,7 +199,7 @@ // assuming both the stack pointer and page_size have their least // significant 2 bits cleared and page_size is a power of 2 final Word alignedMask = Word.fromInt(wordSize() - 1); - final Word stackPointer = asWord(register(stackPointerReg(), wordKind())); + final Word stackPointer = stackPointer(); if (currentMark.minus(stackPointer).and(alignedMask.minus(pageSize())) != Word.zero()) { // Most likely not a recursive lock, go into a slow runtime call log(logEnabled, "+lock{stub}", object); @@ -247,7 +245,7 @@ // a higher level. Second, if the bias was revoked while we held the // lock, the object could not be rebiased toward another thread, so // the bias bit would be clear. - final Word mark = asWord(loadObject(object, 0, markOffset(), true)); + final Word mark = loadWordFromObject(object, markOffset()); if (mark.and(biasedLockMaskInPlace()).toLong() == biasedLockPattern()) { endLockScope(object, false); log(logEnabled, "-lock{bias}", object); @@ -258,7 +256,7 @@ final Word lock = CurrentLockNode.currentLock(wordKind()); // Load displaced mark - final Word displacedMark = loadWord(lock, lockDisplacedMarkOffset()); + final Word displacedMark = loadWordFromWord(lock, lockDisplacedMarkOffset()); if (displacedMark == Word.zero()) { // Recursive locking => done diff -r aa57aa781e86 -r b74402a7079b graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/NewObjectSnippets.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/NewObjectSnippets.java Tue Sep 18 16:58:09 2012 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/NewObjectSnippets.java Tue Sep 18 16:59:24 2012 +0200 @@ -23,7 +23,6 @@ package com.oracle.graal.hotspot.snippets; import static com.oracle.graal.hotspot.nodes.CastFromHub.*; -import static com.oracle.graal.hotspot.nodes.RegisterNode.*; import static com.oracle.graal.hotspot.snippets.HotSpotSnippetUtils.*; import static com.oracle.graal.snippets.SnippetTemplate.Arguments.*; import static com.oracle.graal.snippets.nodes.DirectObjectStoreNode.*; @@ -54,9 +53,9 @@ @Snippet public static Word allocate(@Parameter("size") int size) { - Word thread = asWord(register(threadReg(), wordKind())); - Word top = loadWord(thread, threadTlabTopOffset()); - Word end = loadWord(thread, threadTlabEndOffset()); + Word thread = thread(); + Word top = loadWordFromWord(thread, threadTlabTopOffset()); + Word end = loadWordFromWord(thread, threadTlabEndOffset()); Word newTop = top.plus(size); if (newTop.belowOrEqual(end)) { storeObject(thread, 0, threadTlabTopOffset(), newTop); @@ -169,7 +168,7 @@ * Formats some allocated memory with an object header zeroes out the rest. */ private static void formatObject(Object hub, int size, Word memory, Word compileTimePrototypeMarkWord, boolean fillContents) { - Word prototypeMarkWord = USE_COMPILE_TIME_PROTOTYPE_MARK_WORD ? compileTimePrototypeMarkWord : loadWord(asWord(hub), prototypeMarkWordOffset()); + Word prototypeMarkWord = USE_COMPILE_TIME_PROTOTYPE_MARK_WORD ? compileTimePrototypeMarkWord : loadWordFromObject(hub, prototypeMarkWordOffset()); storeObject(memory, 0, markOffset(), prototypeMarkWord); storeObject(memory, 0, hubOffset(), hub); if (fillContents) { diff -r aa57aa781e86 -r b74402a7079b graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/SnippetIntrinsificationPhase.java --- a/graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/SnippetIntrinsificationPhase.java Tue Sep 18 16:58:09 2012 +0200 +++ b/graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/SnippetIntrinsificationPhase.java Tue Sep 18 16:59:24 2012 +0200 @@ -86,13 +86,14 @@ assert target.getAnnotation(Fold.class) == null; Class< ? >[] parameterTypes = MetaUtil.signatureToTypes(target.signature(), target.holder()); + ResolvedJavaType returnType = (ResolvedJavaType) target.signature().returnType(target.holder()); // Prepare the arguments for the reflective constructor call on the node class. Object[] nodeConstructorArguments = prepareArguments(invoke, parameterTypes, target, false); // Create the new node instance. Class< ? > c = getNodeClass(target, intrinsic); - Node newInstance = createNodeInstance(c, parameterTypes, nodeConstructorArguments); + Node newInstance = createNodeInstance(c, parameterTypes, returnType, intrinsic.setStampFromReturnType(), nodeConstructorArguments); // Replace the invoke with the new node. invoke.node().graph().add(newInstance); @@ -256,7 +257,7 @@ static final int VARARGS = 0x00000080; - private static Node createNodeInstance(Class< ? > nodeClass, Class< ? >[] parameterTypes, Object[] nodeConstructorArguments) { + private static Node createNodeInstance(Class< ? > nodeClass, Class< ? >[] parameterTypes, ResolvedJavaType returnType, boolean setStampFromReturnType, Object[] nodeConstructorArguments) { Object[] arguments = null; Constructor< ? > constructor = null; nextConstructor: @@ -303,7 +304,15 @@ } constructor.setAccessible(true); try { - return (ValueNode) constructor.newInstance(arguments); + ValueNode intrinsicNode = (ValueNode) constructor.newInstance(arguments); + if (setStampFromReturnType) { + if (returnType.kind().isObject()) { + intrinsicNode.setStamp(StampFactory.declared(returnType)); + } else { + intrinsicNode.setStamp(StampFactory.forKind(returnType.kind())); + } + } + return intrinsicNode; } catch (Exception e) { throw new RuntimeException(constructor + Arrays.toString(nodeConstructorArguments), e); }