Skip to content

Commit

Permalink
[GR-58564] Remove Node.NodeIntrinsic.injectedStampIsNonNull method.
Browse files Browse the repository at this point in the history 
PullRequest: graal/18920
  • Loading branch information
@dougxc
dougxc committed Sep 30, 2024
2 parents a8efbf7 + e0fc385 commit 4fb123b
Show file tree
Hide file tree
Showing 9 changed files with 19 additions and 52 deletions.
7 changes: 1 addition & 6 deletions ...ompiler.processor/src/jdk/graal/compiler/replacements/processor/InjectedDependencies.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,9 @@
*/
package jdk.graal.compiler.replacements.processor;

import static jdk.graal.compiler.processor.AbstractProcessor.getAnnotationValue;

import java.util.HashMap;
import java.util.Iterator;

import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.type.DeclaredType;
import javax.lang.model.type.TypeMirror;
Expand Down Expand Up @@ -91,9 +88,7 @@ private InjectedStampDependency() {

@Override
public String getExpression(AbstractProcessor processor, ExecutableElement inject) {
AnnotationMirror nodeIntrinsic = processor.getAnnotation(inject, processor.getType(NodeIntrinsicHandler.NODE_INTRINSIC_CLASS_NAME));
boolean nonNull = nodeIntrinsic != null && getAnnotationValue(nodeIntrinsic, "injectedStampIsNonNull", Boolean.class);
return String.format("injection.getInjectedStamp(%s.class, %s)", GeneratedPlugin.getErasedType(inject.getReturnType()), nonNull);
return String.format("injection.getInjectedStamp(%s.class)", GeneratedPlugin.getErasedType(inject.getReturnType()));
}
}

Expand Down
29 changes: 7 additions & 22 deletions ...ompiler.processor/src/jdk/graal/compiler/replacements/processor/NodeIntrinsicHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ public final class NodeIntrinsicHandler extends AnnotationHandler {
static final String STRUCTURAL_INPUT_CLASS_NAME = "jdk.graal.compiler.nodeinfo.StructuralInput";
static final String RESOLVED_JAVA_TYPE_CLASS_NAME = "jdk.vm.ci.meta.ResolvedJavaType";
static final String VALUE_NODE_CLASS_NAME = "jdk.graal.compiler.nodes.ValueNode";
static final String STAMP_CLASS_NAME = "jdk.graal.compiler.core.common.type.Stamp";
static final String NODE_CLASS_NAME = "jdk.graal.compiler.graph.Node";
static final String NODE_INFO_CLASS_NAME = "jdk.graal.compiler.nodeinfo.NodeInfo";
static final String NODE_INTRINSIC_CLASS_NAME = "jdk.graal.compiler.graph.Node.NodeIntrinsic";
Expand Down Expand Up @@ -110,7 +109,6 @@ public void process(Element element, AnnotationMirror annotation, PluginGenerato
messager.printMessage(Kind.ERROR, "@NodeIntrinsic cannot have a generic return type.", element, annotation);
}

boolean injectedStampIsNonNull = getAnnotationValue(annotation, "injectedStampIsNonNull", Boolean.class);
boolean isFactory = processor.getAnnotation(nodeClass, processor.getType(NODE_INTRINSIC_FACTORY_CLASS_NAME)) != null;

if (returnType.getKind() == TypeKind.VOID) {
Expand Down Expand Up @@ -147,7 +145,7 @@ public void process(Element element, AnnotationMirror annotation, PluginGenerato
TypeMirror[] constructorSignature = constructorSignature(intrinsicMethod);
Map<ExecutableElement, String> nonMatches = new HashMap<>();
if (isFactory) {
List<ExecutableElement> candidates = findIntrinsifyFactoryMethods(factories, constructorSignature, nonMatches, injectedStampIsNonNull);
List<ExecutableElement> candidates = findIntrinsifyFactoryMethods(factories, constructorSignature, nonMatches);
if (checkTooManyElements(annotation, intrinsicMethod, messager, nodeClass, "factories", candidates, msg)) {
return;
}
Expand All @@ -167,7 +165,7 @@ public void process(Element element, AnnotationMirror annotation, PluginGenerato
checkInputType(nodeClass, returnType, element, annotation);
}

List<ExecutableElement> constructors = findConstructors(nodeClass, constructorSignature, nonMatches, injectedStampIsNonNull);
List<ExecutableElement> constructors = findConstructors(nodeClass, constructorSignature, nonMatches);
if (checkTooManyElements(annotation, intrinsicMethod, messager, nodeClass, "constructors", constructors, msg)) {
return;
}
Expand Down Expand Up @@ -255,11 +253,11 @@ private TypeMirror[] constructorSignature(ExecutableElement method) {
return parameters;
}

private List<ExecutableElement> findConstructors(TypeElement nodeClass, TypeMirror[] signature, Map<ExecutableElement, String> nonMatches, boolean requiresInjectedStamp) {
private List<ExecutableElement> findConstructors(TypeElement nodeClass, TypeMirror[] signature, Map<ExecutableElement, String> nonMatches) {
List<ExecutableElement> constructors = ElementFilter.constructorsIn(nodeClass.getEnclosedElements());
List<ExecutableElement> found = new ArrayList<>(constructors.size());
for (ExecutableElement constructor : constructors) {
if (matchSignature(0, constructor, signature, nonMatches, requiresInjectedStamp)) {
if (matchSignature(0, constructor, signature, nonMatches)) {
found.add(constructor);
}
}
Expand Down Expand Up @@ -301,35 +299,26 @@ private static List<ExecutableElement> findIntrinsifyFactoryMethods(TypeElement
return found;
}

private List<ExecutableElement> findIntrinsifyFactoryMethods(List<ExecutableElement> intrinsifyFactoryMethods, TypeMirror[] signature, Map<ExecutableElement, String> nonMatches,
boolean requiresInjectedStamp) {
private List<ExecutableElement> findIntrinsifyFactoryMethods(List<ExecutableElement> intrinsifyFactoryMethods, TypeMirror[] signature, Map<ExecutableElement, String> nonMatches) {
List<ExecutableElement> found = new ArrayList<>(1);
for (ExecutableElement method : intrinsifyFactoryMethods) {
if (matchSignature(1, method, signature, nonMatches, requiresInjectedStamp)) {
if (matchSignature(1, method, signature, nonMatches)) {
found.add(method);
}
}
return found;
}

private boolean matchSignature(int numSkippedParameters, ExecutableElement method, TypeMirror[] signature, Map<ExecutableElement, String> nonMatches, boolean requiresInjectedStamp) {
private boolean matchSignature(int numSkippedParameters, ExecutableElement method, TypeMirror[] signature, Map<ExecutableElement, String> nonMatches) {
int sIdx = 0;
int cIdx = numSkippedParameters;
boolean missingStampArgument = requiresInjectedStamp;
while (cIdx < method.getParameters().size()) {
VariableElement parameter = method.getParameters().get(cIdx++);
TypeMirror paramType = parameter.asType();
if (processor.getAnnotation(parameter, processor.getType(INJECTED_NODE_PARAMETER_CLASS_NAME)) != null) {
if (missingStampArgument && processor.env().getTypeUtils().isSameType(paramType, processor.getType(STAMP_CLASS_NAME))) {
missingStampArgument = false;
}
// skip injected parameters
continue;
}
if (missingStampArgument) {
nonMatches.put(method, String.format("missing injected %s argument", processor.getType(STAMP_CLASS_NAME)));
return false;
}

if (cIdx == method.getParameters().size() && paramType.getKind() == TypeKind.ARRAY) {
TypeMirror varargsType = ((ArrayType) paramType).getComponentType();
Expand All @@ -354,10 +343,6 @@ private boolean matchSignature(int numSkippedParameters, ExecutableElement metho
return false;
}
}
if (missingStampArgument) {
nonMatches.put(method, String.format("missing injected %s argument", processor.getType(STAMP_CLASS_NAME)));
return false;
}

if (sIdx != signature.length) {
nonMatches.put(method, "not enough arguments");
Expand Down
9 changes: 0 additions & 9 deletions compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/graph/Node.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import org.graalvm.collections.EconomicSet;

import jdk.graal.compiler.core.common.Fields;
import jdk.graal.compiler.core.common.type.AbstractPointerStamp;
import jdk.graal.compiler.core.common.type.Stamp;
import jdk.graal.compiler.core.common.util.CompilationAlarm;
import jdk.graal.compiler.debug.Assertions;
Expand Down Expand Up @@ -201,14 +200,6 @@ public abstract class Node implements Cloneable, Formattable {
*/
Class<?> value() default NodeIntrinsic.class;

/**
* If {@code true}, the factory method or constructor selected by the annotation must have
* an {@linkplain InjectedNodeParameter injected} {@link Stamp} parameter. Calling
* {@link AbstractPointerStamp#nonNull()} on the injected stamp is guaranteed to return
* {@code true}.
*/
boolean injectedStampIsNonNull() default false;

/**
* If {@code true} then this is lowered into a node that has side effects.
*/
Expand Down
4 changes: 1 addition & 3 deletions ...piler/src/jdk/graal/compiler/nodes/graphbuilderconf/GeneratedPluginInjectionProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ public interface GeneratedPluginInjectionProvider {
* Gets a stamp denoting a given type and non-nullness property.
*
* @param type the type the returned stamp represents
* @param nonNull specifies if the returned stamp denotes a value that is guaranteed to be
* non-null
*/
Stamp getInjectedStamp(Class<?> type, boolean nonNull);
Stamp getInjectedStamp(Class<?> type);
}
4 changes: 2 additions & 2 deletions ...rc/jdk.graal.compiler/src/jdk/graal/compiler/nodes/graphbuilderconf/GraphBuilderTool.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,13 @@ default DebugContext getDebug() {

@SuppressWarnings("unused")
default boolean canDeferPlugin(GeneratedInvocationPlugin plugin) {
// By default generated plugins must be completely processed during parsing.
// By default, generated plugins must be completely processed during parsing.
return false;
}

@SuppressWarnings("unused")
default boolean shouldDeferPlugin(GeneratedInvocationPlugin plugin) {
// By default generated plugins must be completely processed during parsing.
// By default, generated plugins must be completely processed during parsing.
return false;
}
}
4 changes: 2 additions & 2 deletions compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/DelegatingReplacements.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ public <T> T getInjectedArgument(Class<T> type) {
}

@Override
public Stamp getInjectedStamp(Class<?> type, boolean nonNull) {
return delegate.getInjectedStamp(type, nonNull);
public Stamp getInjectedStamp(Class<?> type) {
return delegate.getInjectedStamp(type);
}

@Override
Expand Down
6 changes: 2 additions & 4 deletions .../jdk.graal.compiler/src/jdk/graal/compiler/replacements/NodeIntrinsificationProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@

public class NodeIntrinsificationProvider implements GeneratedPluginInjectionProvider {

public static final TargetDescription INJECTED_TARGET = null;

private final MetaAccessProvider metaAccess;
private final SnippetReflectionProvider snippetReflection;
private final ForeignCallsProvider foreignCalls;
Expand All @@ -58,14 +56,14 @@ public NodeIntrinsificationProvider(MetaAccessProvider metaAccess, SnippetReflec
}

@Override
public Stamp getInjectedStamp(Class<?> type, boolean nonNull) {
public Stamp getInjectedStamp(Class<?> type) {
JavaKind kind = JavaKind.fromJavaClass(type);
if (kind == JavaKind.Object) {
ResolvedJavaType returnType = metaAccess.lookupJavaType(type);
if (wordTypes.isWord(returnType)) {
return wordTypes.getWordStamp(returnType);
} else {
return StampFactory.object(TypeReference.createWithoutAssumptions(returnType), nonNull);
return StampFactory.object(TypeReference.createWithoutAssumptions(returnType));
}
} else {
return StampFactory.forKind(kind);
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/ReplacementsImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,15 +159,15 @@ public <T> T getInjectedArgument(Class<T> capability) {
}

@Override
public Stamp getInjectedStamp(Class<?> type, boolean nonNull) {
public Stamp getInjectedStamp(Class<?> type) {
JavaKind kind = JavaKind.fromJavaClass(type);
if (kind == JavaKind.Object) {
WordTypes wordTypes = getProviders().getWordTypes();
if (wordTypes.isWord(type)) {
return wordTypes.getWordStamp(type);
} else {
ResolvedJavaType returnType = providers.getMetaAccess().lookupJavaType(type);
return StampFactory.object(TypeReference.createWithoutAssumptions(returnType), nonNull);
return StampFactory.object(TypeReference.createWithoutAssumptions(returnType));
}
} else {
return StampFactory.forKind(kind);
Expand Down
4 changes: 2 additions & 2 deletions ...evm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/meta/SubstrateReplacements.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -394,14 +394,14 @@ public <T> T getInjectedArgument(Class<T> capability) {
}

@Override
public Stamp getInjectedStamp(Class<?> type, boolean nonNull) {
public Stamp getInjectedStamp(Class<?> type) {
JavaKind kind = JavaKind.fromJavaClass(type);
if (kind == JavaKind.Object) {
ResolvedJavaType returnType = providers.getMetaAccess().lookupJavaType(type);
if (providers.getWordTypes().isWord(returnType)) {
return providers.getWordTypes().getWordStamp(returnType);
} else {
return StampFactory.object(TypeReference.createWithoutAssumptions(returnType), nonNull);
return StampFactory.object(TypeReference.createWithoutAssumptions(returnType));
}
} else {
return StampFactory.forKind(kind);
Expand Down

0 comments on commit 4fb123b

Please sign in to comment.