Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GR-50266] [GR-51136] Fix some missing Truffle requirements for Espresso. #8584

Merged
merged 27 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
bf8c663
Fix 2 bugs: - A foreign null does not succeed a cast to primitive - B…
rakachan Nov 21, 2023
96a8933
Frame.getValue(slot) does not work with static slots. For verifying p…
rakachan Nov 21, 2023
74525cb
Check that the object slot of static frame slots is the default value…
rakachan Dec 4, 2023
07af328
Abide by truffle expectations that frames should be clean on entering…
rakachan Dec 6, 2023
82aa6c8
Make static slots tag initialize to 0.
rakachan Jan 3, 2024
ecc4e3c
Exploit the fact that uninitialized static frame slots return 0 on ac…
rakachan Jan 3, 2024
53e0279
Propagate new frame static slot initialization handling changes to th…
rakachan Jan 5, 2024
628900c
Fix typo, and remove unused constants.
rakachan Jan 5, 2024
89326ff
Make some FrameDescriptor fields as immutable during Truffle host com…
rakachan Jan 5, 2024
0f98481
Add possibility to easily remove assertions from subprocess tests. Us…
rakachan Jan 25, 2024
c84d71c
Update FrameHostReadsTest using assertion disabled.
rakachan Jan 25, 2024
060c9b3
Adds an assertion in frame access node to fail early when static slot…
rakachan Jan 25, 2024
1457461
Remove intrinsification of 'isStatic'
rakachan Jan 25, 2024
2cc27f0
More precise handling of static slot transfer during bytecode OSR. St…
rakachan Jan 25, 2024
8f6a411
ClearPrimitiveEffect in FrameAccessVerification uses correct access f…
rakachan Jan 31, 2024
69467ac
Style fixes: use multi-line comment, remove useless whitespace, inver…
rakachan Jan 31, 2024
163c0a0
Adds precision to Frame documentation.
rakachan Jan 31, 2024
69dec2d
Adds profiles to ToEspressoNode.
rakachan Feb 5, 2024
4279466
Add changelog entry for uninitialized static slot behavior.
rakachan Feb 5, 2024
4ed5c6b
restore and deprecate static-mode related fields in FrameDescriptor
rakachan Feb 7, 2024
fa3b87f
Use InlinedBranchProfile instead of regular BranchProfile
rakachan Feb 7, 2024
dd3782a
Use reflection to check for frame assertions disabled. Fixes test cal…
rakachan Feb 15, 2024
4288fbe
Use byte static slot to differentiate uninit/Regular/OSR state, rathe…
rakachan Feb 15, 2024
513f6fe
Remove isStatic check from isObject, and make it an assert for all `i…
rakachan Feb 29, 2024
001be02
Fix frame slot type verification in tests.
rakachan Mar 6, 2024
774f69a
Frame assertions checking is delegated to a helper method. Use GraalE…
rakachan Mar 13, 2024
9475aee
Update header years.
rakachan Mar 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, 2024, 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
Expand All @@ -26,14 +26,9 @@

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.List;
import java.util.concurrent.TimeUnit;

import com.oracle.truffle.api.test.SubprocessTestUtils;
import com.oracle.truffle.runtime.BytecodeOSRMetadata;
import com.oracle.truffle.runtime.OptimizedTruffleRuntime;
import com.oracle.truffle.runtime.OptimizedCallTarget;

import jdk.graal.compiler.test.GraalTest;
import org.graalvm.polyglot.Context;
import org.junit.Assert;
import org.junit.Before;
Expand All @@ -54,10 +49,17 @@
import com.oracle.truffle.api.frame.FrameSlotKind;
import com.oracle.truffle.api.frame.FrameSlotTypeException;
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.impl.FrameWithoutBoxing;
import com.oracle.truffle.api.nodes.BytecodeOSRNode;
import com.oracle.truffle.api.nodes.ExplodeLoop;
import com.oracle.truffle.api.nodes.Node;
import com.oracle.truffle.api.nodes.RootNode;
import com.oracle.truffle.api.test.SubprocessTestUtils;
import com.oracle.truffle.runtime.BytecodeOSRMetadata;
import com.oracle.truffle.runtime.OptimizedCallTarget;
import com.oracle.truffle.runtime.OptimizedTruffleRuntime;

import jdk.graal.compiler.test.GraalTest;

public class BytecodeOSRNodeTest extends TestWithSynchronousCompiling {

Expand Down Expand Up @@ -407,6 +409,24 @@ public void testFrameTransfer() {
*/
@Test
public void testFrameTransferWithStaticAccesses() {
frameTransferWithStaticAccesses();
}

/*
* Test that the frame transfer helper works as expected, even with static accesses, both on OSR
* enter and exit, and even when assertions are disabled.
*/
@Test
public void testFrameTransferWithStaticAccessesWithAssertionsDisabled() throws Throwable {
// Execute in a subprocess to disable assertion checking for frames.
SubprocessTestUtils.executeInSubprocessWithAssertionsDisabled(BytecodeOSRNodeTest.class,
() -> {
Assert.assertFalse(FrameAssertionsChecker.areFrameAssertionsEnabled());
frameTransferWithStaticAccesses();
}, true, List.of(FrameWithoutBoxing.class));
}

public static void frameTransferWithStaticAccesses() {
var frameBuilder = FrameDescriptor.newBuilder();
RootNode rootNode = new Program(new FrameTransferringWithStaticAccessNode(frameBuilder), frameBuilder.build());
OptimizedCallTarget target = (OptimizedCallTarget) rootNode.getCallTarget();
Expand Down Expand Up @@ -456,6 +476,40 @@ public void testFrameTransferWithUninitializedSlots() {
Assert.assertEquals(42, target.call());
}

/*
* Same as above, but with static slots.
*
* Note that uninitialized static slots differs from regular slots in that it may be read as any
* kind, and always return the default ('defaultValue' for Object, '0' for primitives).
*/
@Test
public void testFrameTransferWithUninitializedStaticSlots() {
frameTransferWithUninitializedStaticSlots();
}

/*
* Same as above, but with assertion disabled.
*/
@Test
public void testFrameTransferWithUninitializedStaticSlotsWithoutAssertions() throws Throwable {
// Execute in a subprocess to disable assertion checking for frames.
SubprocessTestUtils.executeInSubprocessWithAssertionsDisabled(BytecodeOSRNodeTest.class,
() -> {
Assert.assertFalse(FrameAssertionsChecker.areFrameAssertionsEnabled());
frameTransferWithUninitializedStaticSlots();
}, true, List.of(FrameWithoutBoxing.class));
}

public static void frameTransferWithUninitializedStaticSlots() {
// use a non-null default value to make sure it gets copied properly.
var frameBuilder = FrameDescriptor.newBuilder();
Object defaultValue = new Object();
frameBuilder.defaultValue(defaultValue);
RootNode rootNode = new Program(new FrameTransferringNodeWithUninitializedStaticSlots(frameBuilder, defaultValue), frameBuilder.build());
OptimizedCallTarget target = (OptimizedCallTarget) rootNode.getCallTarget();
Assert.assertEquals(42, target.call());
}

/*
* Test that we can safely recover from bailing out of OSR compilation while an OSR frame is
* currently executing.
Expand Down Expand Up @@ -1538,6 +1592,71 @@ public void checkOSRState(VirtualFrame frame) {
}
}

public static class FrameTransferringNodeWithUninitializedStaticSlots extends FrameTransferringWithStaticAccessNode {
final Object defaultValue;

public FrameTransferringNodeWithUninitializedStaticSlots(FrameDescriptor.Builder frameDescriptor, Object defaultValue) {
super(frameDescriptor);
this.defaultValue = defaultValue;
}

private void ensureUninitReturnsDefault(VirtualFrame frame, int index) {
assertEquals(defaultValue, frame.getObjectStatic(index));
assertEquals((byte) 0, frame.getByteStatic(index));
assertEquals(false, frame.getBooleanStatic(index));
assertEquals(0, frame.getIntStatic(index));
assertEquals(0L, frame.getLongStatic(index));
assertEquals(0f, frame.getFloatStatic(index));
assertEquals(0d, frame.getDoubleStatic(index));
}

@Override
public void setRegularState(VirtualFrame frame) {
frame.setByteStatic(byteSlot, (byte) 1);
// everything else is uninitialized
}

@Override
public void checkRegularState(VirtualFrame frame) {
try {
assertEquals((byte) 1, frame.getByteStatic(byteSlot));
// these slots are uninitialized
ensureUninitReturnsDefault(frame, booleanSlot);
ensureUninitReturnsDefault(frame, doubleSlot);
ensureUninitReturnsDefault(frame, floatSlot);
ensureUninitReturnsDefault(frame, intSlot);
ensureUninitReturnsDefault(frame, longSlot);
ensureUninitReturnsDefault(frame, objectSlot);
} catch (FrameSlotTypeException ex) {
CompilerDirectives.transferToInterpreter();
throw new IllegalStateException("Error accessing index slot");
}
}

@Override
public void setOSRState(VirtualFrame frame) {
frame.setByteStatic(byteSlot, (byte) 2);
// everything else is uninitialized
}

@Override
public void checkOSRState(VirtualFrame frame) {
try {
assertEquals((byte) 2, frame.getByteStatic(byteSlot));
// these slots are uninitialized
ensureUninitReturnsDefault(frame, booleanSlot);
ensureUninitReturnsDefault(frame, doubleSlot);
ensureUninitReturnsDefault(frame, floatSlot);
ensureUninitReturnsDefault(frame, intSlot);
ensureUninitReturnsDefault(frame, longSlot);
ensureUninitReturnsDefault(frame, objectSlot);
} catch (FrameSlotTypeException ex) {
CompilerDirectives.transferToInterpreter();
throw new IllegalStateException("Error accessing index slot");
}
}
}

public static class OSRDisablingTransferringNode extends FrameTransferringNode {
@CompilationFinal int staticSlot;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright (c) 2024, 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. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* 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 jdk.graal.compiler.truffle.test;

import java.lang.reflect.Field;

import org.junit.Assert;

import com.oracle.truffle.api.impl.FrameWithoutBoxing;

public final class FrameAssertionsChecker {
private static final boolean ASSERTIONS_ENABLED;

private FrameAssertionsChecker() {
}

static {
ASSERTIONS_ENABLED = getFrameAssertionsStatus();
}

public static boolean areFrameAssertionsEnabled() {
return ASSERTIONS_ENABLED;
}

private static boolean getFrameAssertionsStatus() {
Field assertionsEnabledField = null;
for (Field f : FrameWithoutBoxing.class.getDeclaredFields()) {
if (f.getName().equals("ASSERTIONS_ENABLED")) {
assertionsEnabledField = f;
break;
}
}
Assert.assertNotNull(assertionsEnabledField);
try {
assertionsEnabledField.setAccessible(true);
return ((boolean) assertionsEnabledField.get(null));
} catch (IllegalAccessException e) {
throw new RuntimeException(e);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, 2024, 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
Expand All @@ -24,18 +24,23 @@
*/
package jdk.graal.compiler.truffle.test;

import jdk.graal.compiler.api.directives.GraalDirectives;
import jdk.graal.compiler.nodes.FieldLocationIdentity;
import jdk.graal.compiler.nodes.NamedLocationIdentity;
import jdk.graal.compiler.nodes.StructuredGraph;
import jdk.graal.compiler.nodes.memory.ReadNode;
import java.io.IOException;
import java.util.List;

import org.graalvm.word.LocationIdentity;
import org.junit.Assert;
import org.junit.Test;

import com.oracle.truffle.api.Truffle;
import com.oracle.truffle.api.frame.FrameDescriptor;
import com.oracle.truffle.api.impl.FrameWithoutBoxing;
import com.oracle.truffle.api.test.SubprocessTestUtils;

import jdk.graal.compiler.api.directives.GraalDirectives;
import jdk.graal.compiler.nodes.FieldLocationIdentity;
import jdk.graal.compiler.nodes.NamedLocationIdentity;
import jdk.graal.compiler.nodes.StructuredGraph;
import jdk.graal.compiler.nodes.memory.ReadNode;

/**
* Tests that we can move frame array reads out of the loop even if there is a side-effect in the
Expand All @@ -59,11 +64,23 @@ public static int snippet0(FrameWithoutBoxing frame, int index) {
}

@Test
public void test0() {
public void test0() throws IOException, InterruptedException {
// Run in subprocess to disable assertions in FrameWithoutBoxing.
// Assertion checking requires additional checks in the frame descriptor for handling of
// static slots.
SubprocessTestUtils.executeInSubprocessWithAssertionsDisabled(FrameHostReadsTest.class, () -> {
compileAndCheck();
},
true,
List.of(FrameWithoutBoxing.class));
}

private void compileAndCheck() {
getTruffleCompiler();
initAssertionError();
Assert.assertSame("New frame implementation detected. Make sure to update this test.", FrameWithoutBoxing.class,
Truffle.getRuntime().createVirtualFrame(new Object[0], FrameDescriptor.newBuilder().build()).getClass());
Assert.assertTrue("Frame assertions should be disabled.", !FrameAssertionsChecker.areFrameAssertionsEnabled());

StructuredGraph graph = getFinalGraph("snippet0");

Expand All @@ -86,25 +103,21 @@ public void test0() {
}

/*
* Frame read for FrameWithoutBoxing.indexedTags and
* FrameWithoutBoxing.indexedPrimitiveLocals.
*
* Note we expect two reads here as assertions are enabled.
* Frame read for FrameWithoutBoxing.indexedPrimitiveLocals.
*/
Assert.assertEquals(2, fieldReads);
Assert.assertEquals(1, fieldReads);

/*
* Array reads inside of the loop. We expect the loop to get unrolled, so we expect 5 times
* the number of reads. It is important that the loop does not contain reads to
* FrameWithoutBoxing.indexedTags or FrameWithoutBoxing.indexedPrimitiveLocals.
*/
Assert.assertEquals(10, arrayReads);
Assert.assertEquals(5, arrayReads);

/*
* Array.length reads. We also read two because we have assertions enabled. one for
* FrameWithoutBoxing.indexedTags and one for FrameWithoutBoxing.indexedPrimitiveLocals.
* Array.length reads. We read one for FrameWithoutBoxing.indexedPrimitiveLocals.
*/
Assert.assertEquals(2, arrayLengthReads);
Assert.assertEquals(1, arrayLengthReads);
Assert.assertEquals(0, otherReads);
}

Expand All @@ -118,5 +131,4 @@ public Throwable fillInStackTrace() {
}
};
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2023, 2024, 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
Expand Down Expand Up @@ -104,7 +104,6 @@ public class KnownTruffleTypes extends AbstractKnownTruffleTypes {
public final ResolvedJavaField FrameDescriptor_materializeCalled = findField(FrameDescriptor, "materializeCalled");
public final ResolvedJavaField FrameDescriptor_indexedSlotTags = findField(FrameDescriptor, "indexedSlotTags");
public final ResolvedJavaField FrameDescriptor_auxiliarySlotCount = findField(FrameDescriptor, "auxiliarySlotCount");
public final ResolvedJavaField FrameDescriptor_staticMode = findField(FrameDescriptor, "staticMode");

public final ResolvedJavaType FrameSlotKind = lookupTypeCached("com.oracle.truffle.api.frame.FrameSlotKind");
public final ResolvedJavaField FrameSlotKind_Object = findField(FrameSlotKind, "Object");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, 2024, 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
Expand Down Expand Up @@ -34,7 +34,6 @@
import jdk.graal.compiler.phases.PhaseSuite;
import jdk.graal.compiler.phases.common.HighTierLoweringPhase;
import jdk.graal.compiler.phases.tiers.HighTierContext;

import jdk.vm.ci.meta.ResolvedJavaField;
import jdk.vm.ci.meta.ResolvedJavaType;

Expand All @@ -58,10 +57,12 @@ protected void run(StructuredGraph graph, HighTierContext context) {
return;
}
ResolvedJavaType frameType = env.types().FrameWithoutBoxing;
ResolvedJavaType frameDescriptorType = env.types().FrameDescriptor;
for (Node node : graph.getNodes()) {
if (node instanceof LoadFieldNode) {
LoadFieldNode fieldNode = (LoadFieldNode) node;
if (isForcedImmutable(env, fieldNode.field(), frameType) && !fieldNode.getLocationIdentity().isImmutable()) {
if ((isForcedImmutable(env, fieldNode.field(), frameType) || isForcedImmutable(env, fieldNode.field(), frameDescriptorType)) &&
!fieldNode.getLocationIdentity().isImmutable()) {
graph.replaceFixedWithFixed(fieldNode, graph.add(LoadFieldNode.createOverrideImmutable(fieldNode)));
}
}
Expand Down
Loading
Loading