Skip to content

Commit 8fe8455

Browse files
dougxcdougxc
committed
add test to check that Graal stubs do not create and install duplicate HotSpot RuntimeStubs
1 parent 3b8f7ab commit 8fe8455

File tree

11 files changed

+166
-140
lines changed

11 files changed

+166
-140
lines changed

‎compiler/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Compiler Changelog
22

3-
This changelog summarizes newly introduced optimizations that may be relevant to other teams.
3+
This changelog summarizes newly introduced optimizations and other compiler related changes.
44

55
## Version 22.3.0
66
* (GR-19840): An image produced by GraalVM's jlink now includes and uses libgraal by default and its `java -version` output includes GraalVM branding.

‎compiler/src/org.graalvm.compiler.core.common/src/org/graalvm/compiler/core/common/spi/ForeignCallDescriptor.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,12 @@ public class ForeignCallDescriptor {
5050
protected final boolean isGuaranteedSafepoint;
5151
protected final LocationIdentity[] killedLocations;
5252

53-
public ForeignCallDescriptor(String name, Class<?> resultType, Class<?>[] argumentTypes, boolean isReexecutable, LocationIdentity[] killedLocations, boolean canDeoptimize,
53+
public ForeignCallDescriptor(String name,
54+
Class<?> resultType,
55+
Class<?>[] argumentTypes,
56+
boolean isReexecutable,
57+
LocationIdentity[] killedLocations,
58+
boolean canDeoptimize,
5459
boolean isGuaranteedSafepoint) {
5560
this.isReexecutable = isReexecutable;
5661
this.killedLocations = killedLocations;

‎compiler/src/org.graalvm.compiler.core.common/src/org/graalvm/compiler/core/common/spi/ForeignCallSignature.java

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,26 @@
2525
package org.graalvm.compiler.core.common.spi;
2626

2727
import java.util.Arrays;
28+
import java.util.regex.Pattern;
29+
30+
import org.graalvm.compiler.debug.GraalError;
2831

2932
/**
3033
* The name and signature of a {@link ForeignCallDescriptor foreign call}.
3134
*/
3235
public final class ForeignCallSignature {
3336

37+
/**
38+
* {@link ForeignCallSignature} names can only contain non-whitespace characters.
39+
*/
40+
public static final Pattern NAME_PATTERN = Pattern.compile("[\\S]+");
41+
3442
private final String name;
3543
private final Class<?> resultType;
3644
private final Class<?>[] argumentTypes;
3745

3846
public ForeignCallSignature(String name, Class<?> resultType, Class<?>... argumentTypes) {
47+
GraalError.guarantee(NAME_PATTERN.matcher(name).matches(), "invalid foreign call name: %s", name);
3948
this.name = name;
4049
this.resultType = resultType;
4150
this.argumentTypes = argumentTypes;
@@ -76,14 +85,24 @@ public boolean equals(Object obj) {
7685
return false;
7786
}
7887

79-
@Override
80-
public String toString() {
81-
StringBuilder sb = new StringBuilder(name).append('(');
88+
/**
89+
* Gets the signature of this foreign call as a String.
90+
*
91+
* @param withName if true, the {@link #getName()} is prepended to the returned string
92+
*/
93+
public String toString(boolean withName) {
94+
StringBuilder sb = withName ? new StringBuilder(name) : new StringBuilder();
95+
sb.append('(');
8296
String sep = "";
8397
for (Class<?> arg : argumentTypes) {
8498
sb.append(sep).append(arg.getSimpleName());
8599
sep = ",";
86100
}
87101
return sb.append(')').append(resultType.getSimpleName()).toString();
88102
}
103+
104+
@Override
105+
public String toString() {
106+
return toString(true);
107+
}
89108
}

‎compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/CompilationPrinter.java

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030

3131
import org.graalvm.compiler.code.CompilationResult;
3232
import org.graalvm.compiler.core.common.CompilationIdentifier;
33+
import org.graalvm.compiler.core.common.spi.ForeignCallSignature;
34+
import org.graalvm.compiler.debug.GraalError;
3335
import org.graalvm.compiler.debug.TTY;
3436
import org.graalvm.compiler.options.OptionValues;
3537

@@ -42,7 +44,7 @@
4244
public final class CompilationPrinter {
4345

4446
private final CompilationIdentifier id;
45-
private final JavaMethod method;
47+
private final Object source;
4648
private final int entryBCI;
4749
private final long start;
4850
private final long allocatedBytesBefore;
@@ -56,28 +58,30 @@ public final class CompilationPrinter {
5658
*
5759
* @param options used to get the value of {@link GraalCompilerOptions#PrintCompilation}
5860
* @param id the identifier for the compilation
59-
* @param method the method for which code is being compiled
61+
* @param source describes the object for which code is being compiled. Must be a
62+
* {@link JavaMethod} or a {@link ForeignCallSignature}
6063
* @param entryBCI the BCI at which compilation starts
6164
*/
62-
public static CompilationPrinter begin(OptionValues options, CompilationIdentifier id, JavaMethod method, int entryBCI) {
65+
public static CompilationPrinter begin(OptionValues options, CompilationIdentifier id, Object source, int entryBCI) {
66+
GraalError.guarantee(source instanceof JavaMethod || source instanceof ForeignCallSignature, "%s", source.getClass());
6367
if (PrintCompilation.getValue(options) && !TTY.isSuppressed()) {
64-
return new CompilationPrinter(id, method, entryBCI);
68+
return new CompilationPrinter(id, source, entryBCI);
6569
}
6670
return DISABLED;
6771
}
6872

6973
private static final CompilationPrinter DISABLED = new CompilationPrinter();
7074

7175
private CompilationPrinter() {
72-
this.method = null;
76+
this.source = null;
7377
this.id = null;
7478
this.entryBCI = -1;
7579
this.start = -1;
7680
this.allocatedBytesBefore = -1;
7781
}
7882

79-
private CompilationPrinter(CompilationIdentifier id, JavaMethod method, int entryBCI) {
80-
this.method = method;
83+
private CompilationPrinter(CompilationIdentifier id, Object source, int entryBCI) {
84+
this.source = source;
8185
this.id = id;
8286
this.entryBCI = entryBCI;
8387

@@ -86,10 +90,19 @@ private CompilationPrinter(CompilationIdentifier id, JavaMethod method, int entr
8690
}
8791

8892
private String getMethodDescription() {
89-
return String.format("%-30s %-70s %-45s %-50s %s", id.toString(CompilationIdentifier.Verbosity.ID),
90-
method.getDeclaringClass().getName(), method.getName(),
91-
method.getSignature().toMethodDescriptor(),
92-
entryBCI == JVMCICompiler.INVOCATION_ENTRY_BCI ? "" : "(OSR@" + entryBCI + ") ");
93+
if (source instanceof JavaMethod) {
94+
JavaMethod method = (JavaMethod) source;
95+
return String.format("%-30s %-70s %-45s %-50s %s", id.toString(CompilationIdentifier.Verbosity.ID),
96+
method.getDeclaringClass().getName(), method.getName(),
97+
method.getSignature().toMethodDescriptor(),
98+
entryBCI == JVMCICompiler.INVOCATION_ENTRY_BCI ? "" : "(OSR@" + entryBCI + ") ");
99+
} else {
100+
ForeignCallSignature sig = (ForeignCallSignature) source;
101+
return String.format("%-30s %-70s %-45s %-50s %s", id.toString(CompilationIdentifier.Verbosity.ID),
102+
"<stub>", sig.getName(),
103+
sig.toString(false),
104+
"");
105+
}
93106
}
94107

95108
/**

‎compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotInvokeJavaMethodTest.java

Lines changed: 15 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@
2424
*/
2525
package org.graalvm.compiler.hotspot.test;
2626

27-
import org.graalvm.compiler.hotspot.meta.HotSpotHostForeignCallsProvider;
27+
import org.graalvm.compiler.hotspot.meta.HotSpotForeignCallDescriptor;
28+
import org.graalvm.compiler.hotspot.meta.HotSpotHostForeignCallsProvider.TestForeignCalls;
2829
import org.graalvm.compiler.nodes.ValueNode;
2930
import org.graalvm.compiler.nodes.extended.ForeignCallNode;
3031
import org.graalvm.compiler.nodes.graphbuilderconf.GraphBuilderContext;
@@ -41,63 +42,19 @@ public class HotSpotInvokeJavaMethodTest extends HotSpotGraalCompilerTest {
4142

4243
@Override
4344
protected void registerInvocationPlugins(InvocationPlugins invocationPlugins) {
44-
invocationPlugins.register(HotSpotInvokeJavaMethodTest.class, new InvocationPlugin("booleanReturnsBoolean", boolean.class) {
45-
@Override
46-
public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, InvocationPlugin.Receiver receiver, ValueNode arg) {
47-
ForeignCallNode node = new ForeignCallNode(HotSpotHostForeignCallsProvider.TestForeignCalls.BOOLEAN_RETURNS_BOOLEAN, arg);
48-
b.addPush(JavaKind.Boolean, node);
49-
return true;
50-
}
51-
});
52-
invocationPlugins.register(HotSpotInvokeJavaMethodTest.class, new InvocationPlugin("byteReturnsByte", byte.class) {
53-
@Override
54-
public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, InvocationPlugin.Receiver receiver, ValueNode arg) {
55-
ForeignCallNode node = new ForeignCallNode(HotSpotHostForeignCallsProvider.TestForeignCalls.BYTE_RETURNS_BYTE, arg);
56-
b.addPush(JavaKind.Byte, node);
57-
return true;
58-
}
59-
});
60-
invocationPlugins.register(HotSpotInvokeJavaMethodTest.class, new InvocationPlugin("shortReturnsShort", short.class) {
61-
@Override
62-
public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, InvocationPlugin.Receiver receiver, ValueNode arg) {
63-
ForeignCallNode node = new ForeignCallNode(HotSpotHostForeignCallsProvider.TestForeignCalls.SHORT_RETURNS_SHORT, arg);
64-
b.addPush(JavaKind.Short, node);
65-
return true;
66-
}
67-
});
68-
invocationPlugins.register(HotSpotInvokeJavaMethodTest.class, new InvocationPlugin("charReturnsChar", char.class) {
69-
@Override
70-
public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, InvocationPlugin.Receiver receiver, ValueNode arg) {
71-
ForeignCallNode node = new ForeignCallNode(HotSpotHostForeignCallsProvider.TestForeignCalls.CHAR_RETURNS_CHAR, arg);
72-
b.addPush(JavaKind.Char, node);
73-
return true;
74-
}
75-
});
76-
invocationPlugins.register(HotSpotInvokeJavaMethodTest.class, new InvocationPlugin("intReturnsInt", int.class) {
77-
@Override
78-
public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, InvocationPlugin.Receiver receiver, ValueNode arg) {
79-
ForeignCallNode node = new ForeignCallNode(HotSpotHostForeignCallsProvider.TestForeignCalls.INT_RETURNS_INT, arg);
80-
b.addPush(JavaKind.Int, node);
81-
return true;
82-
}
83-
});
84-
invocationPlugins.register(HotSpotInvokeJavaMethodTest.class, new InvocationPlugin("longReturnsLong", long.class) {
85-
@Override
86-
public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, InvocationPlugin.Receiver receiver, ValueNode arg) {
87-
ForeignCallNode node = new ForeignCallNode(HotSpotHostForeignCallsProvider.TestForeignCalls.LONG_RETURNS_LONG, arg);
88-
b.addPush(JavaKind.Long, node);
89-
return true;
90-
}
91-
});
92-
invocationPlugins.register(HotSpotInvokeJavaMethodTest.class, new InvocationPlugin("objectReturnsObject", Object.class) {
93-
@Override
94-
public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, InvocationPlugin.Receiver receiver, ValueNode arg) {
95-
ForeignCallNode node = new ForeignCallNode(HotSpotHostForeignCallsProvider.TestForeignCalls.OBJECT_RETURNS_OBJECT, arg);
96-
b.addPush(JavaKind.Object, node);
97-
return true;
98-
}
99-
});
100-
45+
for (JavaKind kind : TestForeignCalls.KINDS) {
46+
HotSpotForeignCallDescriptor desc = TestForeignCalls.createStubCallDescriptor(kind);
47+
String name = desc.getName();
48+
Class<?> argType = desc.getSignature().getArgumentTypes()[0];
49+
invocationPlugins.register(HotSpotInvokeJavaMethodTest.class, new InvocationPlugin(name, argType) {
50+
@Override
51+
public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, InvocationPlugin.Receiver receiver, ValueNode arg) {
52+
ForeignCallNode node = new ForeignCallNode(desc, arg);
53+
b.addPush(kind, node);
54+
return true;
55+
}
56+
});
57+
}
10158
super.registerInvocationPlugins(invocationPlugins);
10259
}
10360

‎compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotForeignCallDescriptor.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,19 @@ public enum Reexecutability {
9595
private final Transition transition;
9696
private final Reexecutability reexecutability;
9797

98-
public HotSpotForeignCallDescriptor(Transition transition, Reexecutability reexecutability, LocationIdentity[] killedLocations, String name, Class<?> resultType, Class<?>... argumentTypes) {
99-
super(name, resultType, argumentTypes, reexecutability == Reexecutability.REEXECUTABLE, killedLocations, transition == SAFEPOINT, transition == SAFEPOINT);
98+
public HotSpotForeignCallDescriptor(Transition transition,
99+
Reexecutability reexecutability,
100+
LocationIdentity[] killedLocations,
101+
String name,
102+
Class<?> resultType,
103+
Class<?>... argumentTypes) {
104+
super(name,
105+
resultType,
106+
argumentTypes,
107+
reexecutability == Reexecutability.REEXECUTABLE,
108+
killedLocations,
109+
transition == SAFEPOINT,
110+
transition == SAFEPOINT);
100111
this.transition = transition;
101112
this.reexecutability = reexecutability;
102113
}

0 commit comments

Comments
 (0)