Skip to content

[GR-54946] Refactor prior layer method logic. #9497

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Aug 15, 2024
Merged

[GR-54946] Refactor prior layer method logic. #9497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6b7fa50
force resolution of global symbols we define in our code
teshull Aug 1, 2024
afb2c21
Refactor prior layer method logic.
teshull Aug 1, 2024
0397550
fix spelling error
teshull Aug 8, 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
4 changes: 4 additions & 0 deletions substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/HostVM.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,10 @@ public boolean useBaseLayer() {
return false;
}

public boolean analyzedInPriorLayer(@SuppressWarnings("unused") AnalysisMethod method) {
return false;
}

/**
* Check if an {@link AnalysisType} is initialized.
*/
Expand Down
2 changes: 1 addition & 1 deletion ...c/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ protected void apply(boolean forceReparse, Object reason) {
assert !processed : "can only call apply once per MethodTypeFlowBuilder";
processed = true;

if (bb.getHostVM().useBaseLayer() && method.isInBaseLayer()) {
if (method.analyzedInPriorLayer()) {
/*
* We don't need to analyze this method. We already know its return type state from the
* open world analysis. We just install a return flow to link it with its uses.
Expand Down
5 changes: 5 additions & 0 deletions ...vm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,11 @@ protected void initializeBaseLayerMethod(AnalysisMethod analysisMethod, Economic
/* No flags to load in the AnalysisMethod */
}

/**
* Currently we save analysis parsed graphs for methods considered
* {@link AnalysisMethod#isReachable}. See {@link ImageLayerWriter#persistMethodGraphs} for
* implementation.
*/
public boolean hasAnalysisParsedGraph(AnalysisMethod analysisMethod) {
EconomicMap<String, Object> methodData = getMethodData(analysisMethod);
return get(methodData, ANALYSIS_PARSED_GRAPH_TAG) != null;
Expand Down
7 changes: 7 additions & 0 deletions ...tevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ public record Signature(String name, AnalysisType[] parameterTypes) {
private final int id;
/** Marks a method loaded from a base layer. */
private final boolean isInBaseLayer;
private final boolean analyzedInPriorLayer;
private final boolean hasNeverInlineDirective;
private final ExceptionHandler[] exceptionHandlers;
private final LocalVariableTable localVariableTable;
Expand Down Expand Up @@ -226,6 +227,7 @@ protected AnalysisMethod(AnalysisUniverse universe, ResolvedJavaMethod wrapped,
id = universe.computeNextMethodId();
isInBaseLayer = false;
}
analyzedInPriorLayer = isInBaseLayer && universe.hostVM().analyzedInPriorLayer(this);

ExceptionHandler[] original = wrapped.getExceptionHandlers();
exceptionHandlers = new ExceptionHandler[original.length];
Expand Down Expand Up @@ -268,6 +270,7 @@ protected AnalysisMethod(AnalysisMethod original, MultiMethodKey multiMethodKey)
wrapped = original.wrapped;
id = original.id;
isInBaseLayer = original.isInBaseLayer;
analyzedInPriorLayer = original.analyzedInPriorLayer;
declaringClass = original.declaringClass;
signature = original.signature;
hasNeverInlineDirective = original.hasNeverInlineDirective;
Expand Down Expand Up @@ -378,6 +381,10 @@ public boolean isInBaseLayer() {
return isInBaseLayer;
}

public boolean analyzedInPriorLayer() {
return analyzedInPriorLayer;
}

/**
* Registers this method as intrinsified to Graal nodes via a {@link InvocationPlugin graph
* builder plugin}. Such a method is treated similar to an invoked method. For example, method
Expand Down
3 changes: 2 additions & 1 deletion ...ratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,8 @@ public AnalysisMethod resolveConcreteMethod(ResolvedJavaMethod method, ResolvedJ
ResolvedJavaType originalCallerType = originalMethod.getDeclaringClass();

try {
newResolvedMethod = universe.lookup(wrapped.resolveConcreteMethod(originalMethod, originalCallerType));
var concreteMethod = originalMethod instanceof BaseLayerMethod ? originalMethod : wrapped.resolveConcreteMethod(originalMethod, originalCallerType);
newResolvedMethod = universe.lookup(concreteMethod);
if (newResolvedMethod == null) {
newResolvedMethod = getUniverse().getBigbang().fallbackResolveConcreteMethod(this, (AnalysisMethod) method);
}
Expand Down
2 changes: 1 addition & 1 deletion ...src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/StrengthenGraphs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ public final void applyResults(AnalysisMethod method) {
return;
}

if (method.isInBaseLayer()) {
if (method.analyzedInPriorLayer()) {
useSharedLayerGraph(method);
return;
}
Expand Down
28 changes: 20 additions & 8 deletions ...svm.core.graal.aarch64/src/com/oracle/svm/core/graal/aarch64/SubstrateAArch64Backend.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import com.oracle.svm.core.graal.nodes.ComputedIndirectCallTargetNode;
import com.oracle.svm.core.heap.ReferenceAccess;
import com.oracle.svm.core.heap.SubstrateReferenceMapBuilder;
import com.oracle.svm.core.imagelayer.ImageLayerBuildingSupport;
import com.oracle.svm.core.meta.CompressedNullConstant;
import com.oracle.svm.core.meta.SharedField;
import com.oracle.svm.core.meta.SharedMethod;
Expand Down Expand Up @@ -1092,14 +1093,25 @@ private static JavaConstant getZeroConstant(AllocatableValue dst) {
}
}

public AArch64LIRInstruction createLoadMethodPointerConstant(AllocatableValue dst, SubstrateMethodPointerConstant constant) {
if (ImageLayerBuildingSupport.buildingExtensionLayer()) {
if (constant.pointer().getMethod() instanceof SharedMethod sharedMethod && sharedMethod.forceIndirectCall()) {
// GR-53498 AArch64 layered image support
throw VMError.unimplemented("AArch64 does not currently support layered images.");
}
}

return new AArch64LoadMethodPointerConstantOp(dst, constant);
}

@Override
public AArch64LIRInstruction createLoad(AllocatableValue dst, Constant src) {
if (CompressedNullConstant.COMPRESSED_NULL.equals(src)) {
return super.createLoad(dst, getZeroConstant(dst));
} else if (src instanceof CompressibleConstant) {
return loadObjectConstant(dst, (CompressibleConstant) src);
} else if (src instanceof SubstrateMethodPointerConstant) {
return new AArch64LoadMethodPointerConstantOp(dst, (SubstrateMethodPointerConstant) src);
} else if (src instanceof CompressibleConstant constant) {
return loadObjectConstant(dst, constant);
} else if (src instanceof SubstrateMethodPointerConstant constant) {
return createLoadMethodPointerConstant(dst, constant);
}
return super.createLoad(dst, src);
}
Expand All @@ -1108,10 +1120,10 @@ public AArch64LIRInstruction createLoad(AllocatableValue dst, Constant src) {
public LIRInstruction createStackLoad(AllocatableValue dst, Constant src) {
if (CompressedNullConstant.COMPRESSED_NULL.equals(src)) {
return super.createStackLoad(dst, getZeroConstant(dst));
} else if (src instanceof CompressibleConstant) {
return loadObjectConstant(dst, (CompressibleConstant) src);
} else if (src instanceof SubstrateMethodPointerConstant) {
return new AArch64LoadMethodPointerConstantOp(dst, (SubstrateMethodPointerConstant) src);
} else if (src instanceof CompressibleConstant constant) {
return loadObjectConstant(dst, constant);
} else if (src instanceof SubstrateMethodPointerConstant constant) {
return createLoadMethodPointerConstant(dst, constant);
}
return super.createStackLoad(dst, src);
}
Expand Down
33 changes: 22 additions & 11 deletions ...m.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/AMD64CGlobalDataLoadAddressOp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,21 @@
*/
package com.oracle.svm.core.graal.amd64;

import static jdk.vm.ci.code.ValueUtil.asRegister;
import static jdk.graal.compiler.lir.LIRInstruction.OperandFlag.REG;
import static jdk.vm.ci.code.ValueUtil.asRegister;

import jdk.graal.compiler.asm.amd64.AMD64Address;
import jdk.graal.compiler.asm.amd64.AMD64MacroAssembler;
import jdk.graal.compiler.lir.LIRInstructionClass;
import jdk.graal.compiler.lir.amd64.AMD64LIRInstruction;
import jdk.graal.compiler.lir.asm.CompilationResultBuilder;
import org.graalvm.word.Pointer;

import com.oracle.svm.core.SubstrateUtil;
import com.oracle.svm.core.graal.code.CGlobalDataInfo;
import com.oracle.svm.core.graal.code.CGlobalDataReference;

import jdk.graal.compiler.asm.amd64.AMD64Address;
import jdk.graal.compiler.asm.amd64.AMD64MacroAssembler;
import jdk.graal.compiler.lir.LIRInstructionClass;
import jdk.graal.compiler.lir.amd64.AMD64LIRInstruction;
import jdk.graal.compiler.lir.asm.CompilationResultBuilder;
import jdk.vm.ci.code.Register;
import jdk.vm.ci.meta.AllocatableValue;

public final class AMD64CGlobalDataLoadAddressOp extends AMD64LIRInstruction {
Expand All @@ -46,36 +47,46 @@ public final class AMD64CGlobalDataLoadAddressOp extends AMD64LIRInstruction {
@Def(REG) private AllocatableValue result;

private final CGlobalDataInfo dataInfo;
private final int addend;

AMD64CGlobalDataLoadAddressOp(CGlobalDataInfo dataInfo, AllocatableValue result) {
AMD64CGlobalDataLoadAddressOp(CGlobalDataInfo dataInfo, AllocatableValue result, int addend) {
super(TYPE);
assert dataInfo != null;
this.dataInfo = dataInfo;
this.result = result;
this.addend = addend;
}

AMD64CGlobalDataLoadAddressOp(CGlobalDataInfo dataInfo, AllocatableValue result) {
this(dataInfo, result, 0);
}

@Override
public void emitCode(CompilationResultBuilder crb, AMD64MacroAssembler masm) {
Register resultReg = asRegister(result);
if (SubstrateUtil.HOSTED) {
// AOT compilation: record patch that is fixed up later
int before = masm.position();
AMD64Address address = masm.getPlaceholder(before);
if (dataInfo.isSymbolReference()) {
// Pure symbol reference: the data contains the symbol's address, load it
masm.movq(asRegister(result), address);
masm.movq(resultReg, address);
} else {
// Data: load its address
masm.leaq(asRegister(result), address);
masm.leaq(resultReg, address);
}
crb.compilationResult.recordDataPatch(before, new CGlobalDataReference(dataInfo));
} else {
// Runtime compilation: compute the actual address
Pointer globalsBase = CGlobalDataInfo.CGLOBALDATA_RUNTIME_BASE_ADDRESS.get();
Pointer address = globalsBase.add(dataInfo.getOffset());
masm.movq(asRegister(result), address.rawValue());
masm.movq(resultReg, address.rawValue());
if (dataInfo.isSymbolReference()) { // load data, which contains symbol's address
masm.movq(asRegister(result), new AMD64Address(asRegister(result)));
masm.movq(resultReg, new AMD64Address(asRegister(result)));
}
}
if (addend != 0) {
masm.addq(resultReg, addend);
}
}
}
41 changes: 28 additions & 13 deletions ...racle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/SubstrateAMD64Backend.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
import java.util.function.BiConsumer;

import org.graalvm.collections.EconomicMap;
import org.graalvm.collections.Pair;
import org.graalvm.nativeimage.ImageSingletons;

import com.oracle.svm.core.CPUFeatureAccess;
Expand All @@ -62,7 +61,6 @@
import com.oracle.svm.core.deopt.Deoptimizer;
import com.oracle.svm.core.graal.RuntimeCompilation;
import com.oracle.svm.core.graal.code.AssignedLocation;
import com.oracle.svm.core.graal.code.CGlobalDataInfo;
import com.oracle.svm.core.graal.code.PatchConsumerFactory;
import com.oracle.svm.core.graal.code.SharedCompilationResult;
import com.oracle.svm.core.graal.code.StubCallingConvention;
Expand All @@ -88,6 +86,7 @@
import com.oracle.svm.core.heap.ReferenceAccess;
import com.oracle.svm.core.heap.SubstrateReferenceMapBuilder;
import com.oracle.svm.core.imagelayer.DynamicImageLayerInfo;
import com.oracle.svm.core.imagelayer.ImageLayerBuildingSupport;
import com.oracle.svm.core.meta.CompressedNullConstant;
import com.oracle.svm.core.meta.SharedField;
import com.oracle.svm.core.meta.SharedMethod;
Expand Down Expand Up @@ -680,10 +679,10 @@ protected Value emitIndirectForeignCallAddress(ForeignCallLinkage linkage) {
* Load the address for the start of the text section and then add in the offset for
* this specific method.
*/
Pair<CGlobalDataInfo, Integer> methodLocation = DynamicImageLayerInfo.singleton().getPriorLayerMethodLocation(targetMethod);
var methodLocation = DynamicImageLayerInfo.singleton().getPriorLayerMethodLocation(targetMethod);
AllocatableValue basePointerAddress = newVariable(getLIRKindTool().getWordKind());
append(new AMD64CGlobalDataLoadAddressOp(methodLocation.getLeft(), basePointerAddress));
Value codeOffsetInSection = emitConstant(getLIRKindTool().getWordKind(), JavaConstant.forLong(methodLocation.getRight()));
append(new AMD64CGlobalDataLoadAddressOp(methodLocation.base(), basePointerAddress));
Value codeOffsetInSection = emitConstant(getLIRKindTool().getWordKind(), JavaConstant.forLong(methodLocation.offset()));
return getArithmetic().emitAdd(basePointerAddress, codeOffsetInSection, false);
}
if (!shouldEmitOnlyIndirectCalls()) {
Expand Down Expand Up @@ -1462,14 +1461,30 @@ private static JavaConstant getZeroConstant(AllocatableValue dst) {
}
}

public AMD64LIRInstruction createLoadMethodPointerConstant(AllocatableValue dst, SubstrateMethodPointerConstant constant) {
if (ImageLayerBuildingSupport.buildingExtensionLayer()) {
if (constant.pointer().getMethod() instanceof SharedMethod sharedMethod && sharedMethod.forceIndirectCall()) {
/*
* AMD64LoadMethodPointerConstantOp retrieves the address via a PC-relative
* load. This is not possible to do in extension layers when referring to
* methods defined in prior layers.
*/
var methodLocation = DynamicImageLayerInfo.singleton().getPriorLayerMethodLocation(sharedMethod);
return new AMD64CGlobalDataLoadAddressOp(methodLocation.base(), dst, methodLocation.offset());
}
}

return new AMD64LoadMethodPointerConstantOp(dst, constant);
}

@Override
public AMD64LIRInstruction createLoad(AllocatableValue dst, Constant src) {
if (CompressedNullConstant.COMPRESSED_NULL.equals(src)) {
return super.createLoad(dst, getZeroConstant(dst));
} else if (src instanceof CompressibleConstant) {
return loadObjectConstant(dst, (CompressibleConstant) src);
} else if (src instanceof SubstrateMethodPointerConstant) {
return new AMD64LoadMethodPointerConstantOp(dst, (SubstrateMethodPointerConstant) src);
} else if (src instanceof CompressibleConstant constant) {
return loadObjectConstant(dst, constant);
} else if (src instanceof SubstrateMethodPointerConstant constant) {
return createLoadMethodPointerConstant(dst, constant);
}
return super.createLoad(dst, src);
}
Expand All @@ -1478,10 +1493,10 @@ public AMD64LIRInstruction createLoad(AllocatableValue dst, Constant src) {
public LIRInstruction createStackLoad(AllocatableValue dst, Constant src) {
if (CompressedNullConstant.COMPRESSED_NULL.equals(src)) {
return super.createStackLoad(dst, getZeroConstant(dst));
} else if (src instanceof CompressibleConstant) {
return loadObjectConstant(dst, (CompressibleConstant) src);
} else if (src instanceof SubstrateMethodPointerConstant) {
return new AMD64LoadMethodPointerConstantOp(dst, (SubstrateMethodPointerConstant) src);
} else if (src instanceof CompressibleConstant constant) {
return loadObjectConstant(dst, constant);
} else if (src instanceof SubstrateMethodPointerConstant constant) {
return createLoadMethodPointerConstant(dst, constant);
}
return super.createStackLoad(dst, src);
}
Expand Down
7 changes: 5 additions & 2 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ protected void onValueUpdate(EconomicMap<OptionKey<?>, Object> values, String ol
public static final String KEEP_ALIVE_PREFIX = "-keepalive";
private static ValueUpdateHandler<OptimizationLevel> optimizeValueUpdateHandler;
public static OptionEnabledHandler<Boolean> imageLayerEnabledHandler;
public static OptionEnabledHandler<Boolean> imageLayerCreateEnabledHandler;

@Fold
public static boolean getSourceLevelDebug() {
Expand Down Expand Up @@ -433,6 +434,10 @@ public static void setImageLayerEnabledHandler(OptionEnabledHandler<Boolean> upd
SubstrateOptions.imageLayerEnabledHandler = updateHandler;
}

public static void setImageLayerCreateEnabledHandler(OptionEnabledHandler<Boolean> updateHandler) {
SubstrateOptions.imageLayerCreateEnabledHandler = updateHandler;
}

@Option(help = "Track NodeSourcePositions during runtime-compilation")//
public static final HostedOptionKey<Boolean> IncludeNodeSourcePositions = new HostedOptionKey<>(false);

Expand Down Expand Up @@ -856,8 +861,6 @@ public static boolean useLIRBackend() {
*/
@Option(help = "Use linker option to prevent unreferenced symbols in image.")//
public static final HostedOptionKey<Boolean> RemoveUnusedSymbols = new HostedOptionKey<>(OS.getCurrent() != OS.DARWIN);
@Option(help = "Ignore undefined symbols referenced from the built image.")//
public static final HostedOptionKey<Boolean> IgnoreUndefinedReferences = new HostedOptionKey<>(false);
@Option(help = "Use linker option to remove all local symbols from image.")//
public static final HostedOptionKey<Boolean> DeleteLocalSymbols = new HostedOptionKey<>(true);
@Option(help = "Compatibility option to make symbols used for the image heap global. " +
Expand Down
8 changes: 3 additions & 5 deletions ...m/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/snippets/NonSnippetLowerings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,13 @@
import java.util.Map;
import java.util.function.Predicate;

import org.graalvm.collections.Pair;
import org.graalvm.word.LocationIdentity;

import com.oracle.svm.core.FrameAccess;
import com.oracle.svm.core.SubstrateOptions;
import com.oracle.svm.core.SubstrateUtil;
import com.oracle.svm.core.c.BoxedRelocatedPointer;
import com.oracle.svm.core.config.ConfigurationValues;
import com.oracle.svm.core.graal.code.CGlobalDataInfo;
import com.oracle.svm.core.graal.code.SubstrateBackend;
import com.oracle.svm.core.graal.meta.KnownOffsets;
import com.oracle.svm.core.graal.meta.RuntimeConfiguration;
Expand Down Expand Up @@ -384,10 +382,10 @@ public void lower(FixedNode node, LoweringTool tool) {
* address offset of the text section start and then add in the offset for
* this specific method.
*/
Pair<CGlobalDataInfo, Integer> methodLocation = DynamicImageLayerInfo.singleton().getPriorLayerMethodLocation(targetMethod);
var methodLocation = DynamicImageLayerInfo.singleton().getPriorLayerMethodLocation(targetMethod);
AddressNode methodPointerAddress = graph.addOrUniqueWithInputs(
new OffsetAddressNode(new CGlobalDataLoadAddressNode(methodLocation.getLeft()),
ConstantNode.forIntegerKind(ConfigurationValues.getWordKind(), methodLocation.getRight())));
new OffsetAddressNode(new CGlobalDataLoadAddressNode(methodLocation.base()),
ConstantNode.forIntegerKind(ConfigurationValues.getWordKind(), methodLocation.offset())));
loweredCallTarget = createIndirectCall(graph, callTarget, parameters, method, signature, callType, invokeKind, methodPointerAddress);
} else if (!SubstrateBackend.shouldEmitOnlyIndirectCalls()) {
loweredCallTarget = createDirectCall(graph, callTarget, parameters, signature, callType, invokeKind, targetMethod, node);
Expand Down
Loading