Skip to content

Commit

Permalink
Remove unncessary SnippetReflectionProvider arguments and fields
Browse files Browse the repository at this point in the history
  • Loading branch information
Christian Wimmer committed Jan 27, 2024
1 parent c2050e7 commit 120d2dc
Show file tree
Hide file tree
Showing 55 changed files with 380 additions and 463 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,19 @@

import java.util.BitSet;

import org.junit.Assume;
import org.junit.Before;
import org.junit.Test;

import jdk.graal.compiler.api.replacements.Snippet;
import jdk.graal.compiler.asm.amd64.AMD64Address;
import jdk.graal.compiler.asm.amd64.AMD64MacroAssembler;
import jdk.graal.compiler.bytecode.BytecodeProvider;
import jdk.graal.compiler.core.test.GraalCompilerTest;
import jdk.graal.compiler.lir.jtt.LIRTest;
import jdk.graal.compiler.lir.jtt.LIRTestSpecification;
import jdk.graal.compiler.core.common.CompilationIdentifier;
import jdk.graal.compiler.core.common.LIRKind;
import jdk.graal.compiler.core.common.spi.ForeignCallSignature;
import jdk.graal.compiler.core.common.type.DataPointerConstant;
import jdk.graal.compiler.core.test.GraalCompilerTest;
import jdk.graal.compiler.debug.DebugContext;
import jdk.graal.compiler.hotspot.HotSpotBackend;
import jdk.graal.compiler.hotspot.HotSpotForeignCallLinkage;
Expand All @@ -55,6 +57,8 @@
import jdk.graal.compiler.lir.asm.ArrayDataPointerConstant;
import jdk.graal.compiler.lir.asm.CompilationResultBuilder;
import jdk.graal.compiler.lir.gen.LIRGeneratorTool;
import jdk.graal.compiler.lir.jtt.LIRTest;
import jdk.graal.compiler.lir.jtt.LIRTestSpecification;
import jdk.graal.compiler.nodes.StructuredGraph;
import jdk.graal.compiler.nodes.extended.ForeignCallNode;
import jdk.graal.compiler.nodes.graphbuilderconf.GraphBuilderConfiguration;
Expand All @@ -64,10 +68,6 @@
import jdk.graal.compiler.options.OptionValues;
import jdk.graal.compiler.replacements.ReplacementsImpl;
import jdk.graal.compiler.replacements.classfile.ClassfileBytecodeProvider;
import org.junit.Assume;
import org.junit.Before;
import org.junit.Test;

import jdk.vm.ci.amd64.AMD64;
import jdk.vm.ci.amd64.AMD64.CPUFeature;
import jdk.vm.ci.amd64.AMD64Kind;
Expand Down Expand Up @@ -200,7 +200,7 @@ protected StructuredGraph buildInitialGraph(DebugContext debug, CompilationIdent
// Build the snippet graph directly since snippet registration is closed at this point.
ReplacementsImpl d = (ReplacementsImpl) providers.getReplacements();
MetaAccessProvider metaAccess = d.getProviders().getMetaAccess();
BytecodeProvider bytecodes = new ClassfileBytecodeProvider(metaAccess, d.snippetReflection, ClassLoader.getSystemClassLoader());
BytecodeProvider bytecodes = new ClassfileBytecodeProvider(metaAccess, d.getProviders().getSnippetReflection(), ClassLoader.getSystemClassLoader());
return d.makeGraph(debug, bytecodes, method, args, nonNullParameters, null, false, null);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@
import java.lang.reflect.Executable;
import java.lang.reflect.Field;

import org.junit.Assert;
import org.junit.Test;

import jdk.graal.compiler.api.replacements.SnippetReflectionProvider;
import jdk.graal.compiler.core.test.GraalCompilerTest;
import jdk.graal.compiler.replacements.ReplacementsImpl;
import jdk.graal.compiler.replacements.classfile.ClassfileBytecodeProvider;
import org.junit.Assert;
import org.junit.Test;

import jdk.vm.ci.meta.JavaConstant;
import jdk.vm.ci.meta.JavaKind;
import jdk.vm.ci.meta.MetaAccessProvider;
Expand All @@ -50,7 +50,7 @@ public class ReplacementsTest extends GraalCompilerTest {
protected final ClassfileBytecodeProvider getSystemClassLoaderBytecodeProvider() {
ReplacementsImpl d = (ReplacementsImpl) getReplacements();
MetaAccessProvider metaAccess = d.getProviders().getMetaAccess();
ClassfileBytecodeProvider bytecodeProvider = new ClassfileBytecodeProvider(metaAccess, d.snippetReflection, ClassLoader.getSystemClassLoader());
ClassfileBytecodeProvider bytecodeProvider = new ClassfileBytecodeProvider(metaAccess, d.getProviders().getSnippetReflection(), ClassLoader.getSystemClassLoader());
return bytecodeProvider;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public abstract class SnippetsTest extends ReplacementsTest {
protected SnippetsTest() {
ReplacementsImpl d = (ReplacementsImpl) getReplacements();
bytecodeProvider = getSystemClassLoaderBytecodeProvider();
installer = new ReplacementsImpl(null, d.getProviders(), d.snippetReflection, bytecodeProvider, d.target) {
installer = new ReplacementsImpl(null, d.getProviders(), bytecodeProvider, d.target) {

@Override
protected GraphMaker createGraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod original) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
*/
package jdk.graal.compiler.hotspot;

import static jdk.vm.ci.services.Services.IS_IN_NATIVE_IMAGE;
import static jdk.graal.compiler.hotspot.HotSpotReplacementsImpl.isGraalClass;
import static jdk.graal.compiler.nodes.graphbuilderconf.IntrinsicContext.CompilationContext.INLINE_AFTER_PARSING;
import static jdk.vm.ci.services.Services.IS_IN_NATIVE_IMAGE;

import java.lang.reflect.Executable;
import java.lang.reflect.Field;
Expand All @@ -35,6 +35,7 @@
import java.util.function.Consumer;

import org.graalvm.collections.UnmodifiableEconomicMap;

import jdk.graal.compiler.api.replacements.SnippetReflectionProvider;
import jdk.graal.compiler.bytecode.BytecodeProvider;
import jdk.graal.compiler.bytecode.ResolvedJavaMethodBytecode;
Expand All @@ -60,7 +61,6 @@
import jdk.graal.compiler.replacements.ConstantBindingParameterPlugin;
import jdk.graal.compiler.replacements.PEGraphDecoder;
import jdk.graal.compiler.replacements.PartialIntrinsicCallTargetNode;

import jdk.vm.ci.meta.JavaConstant;
import jdk.vm.ci.meta.JavaKind;
import jdk.vm.ci.meta.MetaAccessProvider;
Expand Down Expand Up @@ -297,7 +297,7 @@ static StructuredGraph decodeSnippetGraph(SymbolicEncodedGraph encodedGraph, Res
ParameterPlugin parameterPlugin = null;
if (args != null) {
MetaAccessProvider meta = HotSpotReplacementsImpl.noticeTypes(providers.getMetaAccess());
SnippetReflectionProvider snippetReflection = replacements.snippetReflection;
SnippetReflectionProvider snippetReflection = replacements.getProviders().getSnippetReflection();
if (IS_IN_NATIVE_IMAGE) {
snippetReflection = new LibGraalSnippetReflectionProvider(snippetReflection);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ protected HotSpotMetaAccessExtensionProvider createMetaAccessExtensionProvider()
return new HotSpotMetaAccessExtensionProvider();
}

protected HotSpotReplacementsImpl createReplacements(TargetDescription target, HotSpotProviders p, HotSpotSnippetReflectionProvider snippetReflection, BytecodeProvider bytecodeProvider) {
return new HotSpotReplacementsImpl(p, snippetReflection, bytecodeProvider, target);
protected HotSpotReplacementsImpl createReplacements(TargetDescription target, HotSpotProviders p, BytecodeProvider bytecodeProvider) {
return new HotSpotReplacementsImpl(p, bytecodeProvider, target);
}

protected ClassfileBytecodeProvider createBytecodeProvider(MetaAccessProvider metaAccess, HotSpotSnippetReflectionProvider snippetReflection) {
Expand Down Expand Up @@ -193,7 +193,7 @@ public final HotSpotBackend createBackend(HotSpotGraalRuntimeProvider graalRunti
snippetReflection, wordTypes, stampProvider, platformConfigurationProvider, metaAccessExtensionProvider, loopsDataProvider, config);
HotSpotReplacementsImpl replacements;
try (InitTimer rt = timer("create Replacements provider")) {
replacements = createReplacements(target, providers, snippetReflection, bytecodeProvider);
replacements = createReplacements(target, providers, bytecodeProvider);
providers = replacements.getProviders();
replacements.maybeInitializeEncoder();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,16 @@

import java.util.BitSet;

import jdk.graal.compiler.hotspot.meta.HotSpotProviders;
import jdk.graal.compiler.hotspot.meta.HotSpotWordOperationPlugin;
import org.graalvm.collections.EconomicSet;
import jdk.graal.compiler.api.replacements.SnippetReflectionProvider;

import jdk.graal.compiler.bytecode.Bytecode;
import jdk.graal.compiler.bytecode.BytecodeProvider;
import jdk.graal.compiler.bytecode.ResolvedJavaMethodBytecode;
import jdk.graal.compiler.debug.DebugContext;
import jdk.graal.compiler.debug.GraalError;
import jdk.graal.compiler.graph.NodeSourcePosition;
import jdk.graal.compiler.hotspot.meta.HotSpotProviders;
import jdk.graal.compiler.hotspot.meta.HotSpotWordOperationPlugin;
import jdk.graal.compiler.hotspot.word.HotSpotOperation;
import jdk.graal.compiler.java.GraphBuilderPhase.Instance;
import jdk.graal.compiler.nodes.Invoke;
Expand All @@ -59,7 +59,6 @@
import jdk.graal.compiler.printer.GraalDebugHandlersFactory;
import jdk.graal.compiler.replacements.IntrinsicGraphBuilder;
import jdk.graal.compiler.replacements.ReplacementsImpl;

import jdk.vm.ci.code.TargetDescription;
import jdk.vm.ci.common.NativeImageReinitialize;
import jdk.vm.ci.meta.MetaAccessProvider;
Expand All @@ -71,12 +70,12 @@
* them.
*/
public class HotSpotReplacementsImpl extends ReplacementsImpl {
public HotSpotReplacementsImpl(HotSpotProviders providers, SnippetReflectionProvider snippetReflection, BytecodeProvider bytecodeProvider, TargetDescription target) {
super(new GraalDebugHandlersFactory(snippetReflection), providers, snippetReflection, bytecodeProvider, target);
public HotSpotReplacementsImpl(HotSpotProviders providers, BytecodeProvider bytecodeProvider, TargetDescription target) {
super(new GraalDebugHandlersFactory(providers.getSnippetReflection()), providers, bytecodeProvider, target);
}

HotSpotReplacementsImpl(HotSpotReplacementsImpl replacements, HotSpotProviders providers) {
super(new GraalDebugHandlersFactory(replacements.snippetReflection), providers, replacements.snippetReflection,
super(new GraalDebugHandlersFactory(replacements.getProviders().getSnippetReflection()), providers,
replacements.getDefaultReplacementBytecodeProvider(), replacements.target);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,10 @@
import java.util.Set;
import java.util.function.BiFunction;

import jdk.graal.compiler.hotspot.meta.HotSpotProviders;
import jdk.graal.compiler.hotspot.word.HotSpotWordTypes;
import org.graalvm.collections.EconomicMap;
import org.graalvm.collections.EconomicSet;
import org.graalvm.collections.MapCursor;

import jdk.graal.compiler.api.replacements.Fold;
import jdk.graal.compiler.api.replacements.Snippet;
import jdk.graal.compiler.api.replacements.SnippetReflectionProvider;
Expand All @@ -63,7 +62,9 @@
import jdk.graal.compiler.graph.Node;
import jdk.graal.compiler.graph.NodeMap;
import jdk.graal.compiler.graph.NodeSourcePosition;
import jdk.graal.compiler.hotspot.meta.HotSpotProviders;
import jdk.graal.compiler.hotspot.stubs.ForeignCallStub;
import jdk.graal.compiler.hotspot.word.HotSpotWordTypes;
import jdk.graal.compiler.java.BytecodeParser;
import jdk.graal.compiler.java.GraphBuilderPhase;
import jdk.graal.compiler.nodeinfo.Verbosity;
Expand Down Expand Up @@ -108,7 +109,6 @@
import jdk.graal.compiler.replacements.SnippetTemplate;
import jdk.graal.compiler.replacements.classfile.ClassfileBytecode;
import jdk.graal.compiler.word.WordTypes;

import jdk.vm.ci.code.Architecture;
import jdk.vm.ci.code.TargetDescription;
import jdk.vm.ci.hotspot.HotSpotJVMCIRuntime;
Expand Down Expand Up @@ -247,25 +247,19 @@ static final class SnippetCounterPlugin implements NodePlugin {
String snippetCounterName = 'L' + SnippetCounter.class.getName().replace('.', '/') + ';';
String snippetIntegerHistogramName = 'L' + SnippetIntegerHistogram.class.getName().replace('.', '/') + ';';

private final ReplacementsImpl snippetReplacements;

private SnippetCounterPlugin(ReplacementsImpl snippetReplacements) {
this.snippetReplacements = snippetReplacements;
}

@Override
public boolean handleLoadField(GraphBuilderContext b, ValueNode object, ResolvedJavaField field) {
if (field.getName().equals("group") && field.getDeclaringClass().getName().equals(snippetCounterName)) {
b.addPush(JavaKind.Object, ConstantNode.forConstant(JavaConstant.NULL_POINTER, b.getMetaAccess()));
return true;
}
if (field.getType().getName().equals(snippetCounterName)) {
b.addPush(JavaKind.Object, ConstantNode.forConstant(snippetReplacements.snippetReflection.forObject(SnippetCounter.DISABLED_COUNTER), b.getMetaAccess()));
b.addPush(JavaKind.Object, ConstantNode.forConstant(b.getSnippetReflection().forObject(SnippetCounter.DISABLED_COUNTER), b.getMetaAccess()));
return true;
}

if (field.getType().getName().equals(snippetIntegerHistogramName)) {
b.addPush(JavaKind.Object, ConstantNode.forConstant(snippetReplacements.snippetReflection.forObject(SnippetIntegerHistogram.DISABLED_COUNTER), b.getMetaAccess()));
b.addPush(JavaKind.Object, ConstantNode.forConstant(b.getSnippetReflection().forObject(SnippetIntegerHistogram.DISABLED_COUNTER), b.getMetaAccess()));
return true;
}
return false;
Expand Down Expand Up @@ -366,14 +360,13 @@ private boolean verifySnippetEncodeDecode(DebugContext debug, ResolvedJavaMethod

HotSpotProviders originalProvider = originalReplacements.getProviders();

SnippetReflectionProvider snippetReflection = originalProvider.getSnippetReflection();
SymbolicSnippetEncoder.HotSpotSubstrateConstantReflectionProvider constantReflection = new SymbolicSnippetEncoder.HotSpotSubstrateConstantReflectionProvider(
originalProvider.getConstantReflection());
HotSpotProviders newProviders = new HotSpotProviders(originalProvider.getMetaAccess(), originalProvider.getCodeCache(), constantReflection,
originalProvider.getConstantFieldProvider(), originalProvider.getForeignCalls(), originalProvider.getLowerer(), null, originalProvider.getSuites(),
originalProvider.getRegisters(), snippetReflection, originalProvider.getWordTypes(), originalProvider.getStampProvider(),
originalProvider.getRegisters(), originalProvider.getSnippetReflection(), originalProvider.getWordTypes(), originalProvider.getStampProvider(),
originalProvider.getPlatformConfigurationProvider(), originalProvider.getMetaAccessExtensionProvider(), originalProvider.getLoopsDataProvider(), originalProvider.getConfig());
HotSpotSnippetReplacementsImpl filteringReplacements = new HotSpotSnippetReplacementsImpl(newProviders, snippetReflection,
HotSpotSnippetReplacementsImpl filteringReplacements = new HotSpotSnippetReplacementsImpl(newProviders,
originalProvider.getReplacements().getDefaultReplacementBytecodeProvider(), originalProvider.getCodeCache().getTarget());
filteringReplacements.setGraphBuilderPlugins(originalProvider.getReplacements().getGraphBuilderPlugins());
try (DebugContext.Scope scope = debug.scope("VerifySnippetEncodeDecode", graph)) {
Expand Down Expand Up @@ -418,7 +411,7 @@ private synchronized EncodedSnippets encodeSnippets(OptionValues options) {
snippetReplacements.setGraphBuilderPlugins(copy);
copy.clearInlineInvokePlugins();
copy.appendInlineInvokePlugin(new SnippetInlineInvokePlugin());
copy.appendNodePlugin(new SnippetCounterPlugin(snippetReplacements));
copy.appendNodePlugin(new SnippetCounterPlugin());

EconomicMap<SnippetKey, StructuredGraph> preparedSnippetGraphs = EconomicMap.create();
MapCursor<SnippetKey, BiFunction<OptionValues, HotSpotSnippetReplacementsImpl, StructuredGraph>> cursor = pendingSnippetGraphs.getEntries();
Expand Down Expand Up @@ -979,8 +972,8 @@ class HotSpotSnippetReplacementsImpl extends HotSpotReplacementsImpl {
super(replacements, providers);
}

HotSpotSnippetReplacementsImpl(HotSpotProviders providers, SnippetReflectionProvider snippetReflection, BytecodeProvider bytecodeProvider, TargetDescription target) {
super(providers, snippetReflection, bytecodeProvider, target);
HotSpotSnippetReplacementsImpl(HotSpotProviders providers, BytecodeProvider bytecodeProvider, TargetDescription target) {
super(providers, bytecodeProvider, target);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ public void setProviders(Providers providers) {
}

protected Providers providers;
public final SnippetReflectionProvider snippetReflection;
public final TargetDescription target;
protected GraphBuilderConfiguration.Plugins graphBuilderPlugins;
private final DebugHandlersFactory debugHandlersFactory;
Expand Down Expand Up @@ -151,7 +150,7 @@ public <T> T getInjectedArgument(Class<T> capability) {
return (T) getProviders().getForeignCalls();
}
if (capability.equals(SnippetReflectionProvider.class)) {
return (T) snippetReflection;
return (T) getProviders().getSnippetReflection();
}
if (capability.isAssignableFrom(WordTypes.class)) {
return (T) getProviders().getWordTypes();
Expand Down Expand Up @@ -237,10 +236,9 @@ public void notifyNotInlined(GraphBuilderContext b, ResolvedJavaMethod method, I
private final EconomicMap<String, SnippetTemplateCache> snippetTemplateCache;

@SuppressWarnings("this-escape")
public ReplacementsImpl(DebugHandlersFactory debugHandlersFactory, Providers providers, SnippetReflectionProvider snippetReflection, BytecodeProvider bytecodeProvider,
public ReplacementsImpl(DebugHandlersFactory debugHandlersFactory, Providers providers, BytecodeProvider bytecodeProvider,
TargetDescription target) {
this.providers = providers.copyWith(this);
this.snippetReflection = snippetReflection;
this.target = target;
this.graphs = new ConcurrentHashMap<>();
this.snippetTemplateCache = EconomicMap.create(Equivalence.DEFAULT);
Expand Down Expand Up @@ -536,7 +534,7 @@ protected StructuredGraph buildInitialGraph(DebugContext debug, BytecodeProvider
Plugins plugins = new Plugins(replacements.graphBuilderPlugins);
GraphBuilderConfiguration config = GraphBuilderConfiguration.getSnippetDefault(plugins);
if (args != null) {
plugins.prependParameterPlugin(new ConstantBindingParameterPlugin(args, metaAccess, replacements.snippetReflection));
plugins.prependParameterPlugin(new ConstantBindingParameterPlugin(args, metaAccess, replacements.getProviders().getSnippetReflection()));
}
if (nonNullParameters != null && !nonNullParameters.isEmpty()) {
plugins.appendParameterPlugin(new NonNullParameterPlugin(nonNullParameters));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ public void initialize(TruffleCompilable compilable, boolean firstInitialization
// Hook for SVM
protected TruffleTier newTruffleTier(OptionValues options) {
return new TruffleTier(options, partialEvaluator,
new InstrumentationSuite(partialEvaluator.instrumentationCfg, config.snippetReflection(), partialEvaluator.getInstrumentation()),
new InstrumentationSuite(partialEvaluator.instrumentationCfg, partialEvaluator.getInstrumentation()),
new PostPartialEvaluationSuite(options, TruffleCompilerOptions.IterativePartialEscape.getValue(options)));
}

Expand Down
Loading

0 comments on commit 120d2dc

Please sign in to comment.