From 25a9f1d01892394d9178c0ade24cf595cd12ac2b Mon Sep 17 00:00:00 2001 From: Tom Shull Date: Tue, 18 Jan 2022 12:33:32 +0100 Subject: [PATCH] Set parameters for native abi correctly. --- .../core/aarch64/AArch64NodeLIRBuilder.java | 7 --- .../compiler/core/gen/NodeLIRBuilder.java | 14 ++++- .../aarch64/AArch64HotSpotNodeLIRBuilder.java | 21 +------ .../amd64/AMD64HotSpotNodeLIRBuilder.java | 24 +------- .../aarch64/SubstrateAArch64Backend.java | 32 +++++++++++ .../SubstrateAArch64RegisterConfig.java | 57 ++++++++++++++----- .../graal/amd64/SubstrateAMD64Backend.java | 30 ++++++++++ .../amd64/SubstrateAMD64RegisterConfig.java | 17 ++++-- 8 files changed, 135 insertions(+), 67 deletions(-) diff --git a/compiler/src/org.graalvm.compiler.core.aarch64/src/org/graalvm/compiler/core/aarch64/AArch64NodeLIRBuilder.java b/compiler/src/org.graalvm.compiler.core.aarch64/src/org/graalvm/compiler/core/aarch64/AArch64NodeLIRBuilder.java index 3ebb8c392fec..a938856939a8 100644 --- a/compiler/src/org.graalvm.compiler.core.aarch64/src/org/graalvm/compiler/core/aarch64/AArch64NodeLIRBuilder.java +++ b/compiler/src/org.graalvm.compiler.core.aarch64/src/org/graalvm/compiler/core/aarch64/AArch64NodeLIRBuilder.java @@ -49,11 +49,4 @@ protected boolean peephole(ValueNode valueNode) { public AArch64LIRGenerator getLIRGeneratorTool() { return (AArch64LIRGenerator) super.getLIRGeneratorTool(); } - - @Override - protected void emitPrologue(StructuredGraph graph) { - // XXX Maybe we need something like this. - // getLIRGeneratorTool().emitLoadConstantTableBase(); - super.emitPrologue(graph); - } } diff --git a/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/gen/NodeLIRBuilder.java b/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/gen/NodeLIRBuilder.java index 8bd5c9d86269..8f52854a6898 100644 --- a/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/gen/NodeLIRBuilder.java +++ b/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/gen/NodeLIRBuilder.java @@ -516,7 +516,15 @@ protected void emitPrologue(StructuredGraph graph) { CallingConvention incomingArguments = gen.getResult().getCallingConvention(); Value[] params = new Value[incomingArguments.getArgumentCount()]; - for (int i = 0; i < params.length; i++) { + prologAssignParams(incomingArguments, params); + + gen.emitIncomingValues(params); + + prologSetParameterNodes(graph, params); + } + + protected final void prologAssignParams(CallingConvention incomingArguments, Value[] params) { + for (int i = 0; i < incomingArguments.getArgumentCount(); i++) { params[i] = incomingArguments.getArgument(i); if (ValueUtil.isStackSlot(params[i])) { StackSlot slot = ValueUtil.asStackSlot(params[i]); @@ -525,9 +533,9 @@ protected void emitPrologue(StructuredGraph graph) { } } } + } - gen.emitIncomingValues(params); - + protected void prologSetParameterNodes(StructuredGraph graph, Value[] params) { for (ParameterNode param : graph.getNodes(ParameterNode.TYPE)) { Value paramValue = params[param.index()]; assert paramValue.getValueKind().equals(getLIRGeneratorTool().getLIRKind(param.stamp(NodeView.DEFAULT))) : paramValue + " " + diff --git a/compiler/src/org.graalvm.compiler.hotspot.aarch64/src/org/graalvm/compiler/hotspot/aarch64/AArch64HotSpotNodeLIRBuilder.java b/compiler/src/org.graalvm.compiler.hotspot.aarch64/src/org/graalvm/compiler/hotspot/aarch64/AArch64HotSpotNodeLIRBuilder.java index b880d434dd01..4d6c897855a2 100644 --- a/compiler/src/org.graalvm.compiler.hotspot.aarch64/src/org/graalvm/compiler/hotspot/aarch64/AArch64HotSpotNodeLIRBuilder.java +++ b/compiler/src/org.graalvm.compiler.hotspot.aarch64/src/org/graalvm/compiler/hotspot/aarch64/AArch64HotSpotNodeLIRBuilder.java @@ -25,7 +25,6 @@ package org.graalvm.compiler.hotspot.aarch64; import static jdk.vm.ci.aarch64.AArch64.lr; -import static jdk.vm.ci.code.ValueUtil.isStackSlot; import static jdk.vm.ci.hotspot.aarch64.AArch64HotSpotRegisterConfig.fp; import static jdk.vm.ci.hotspot.aarch64.AArch64HotSpotRegisterConfig.inlineCacheRegister; import static jdk.vm.ci.hotspot.aarch64.AArch64HotSpotRegisterConfig.metaspaceMethodRegister; @@ -52,7 +51,6 @@ import org.graalvm.compiler.nodes.FullInfopointNode; import org.graalvm.compiler.nodes.IndirectCallTargetNode; import org.graalvm.compiler.nodes.NodeView; -import org.graalvm.compiler.nodes.ParameterNode; import org.graalvm.compiler.nodes.SafepointNode; import org.graalvm.compiler.nodes.StructuredGraph; import org.graalvm.compiler.nodes.ValueNode; @@ -63,8 +61,6 @@ import jdk.vm.ci.code.CallingConvention; import jdk.vm.ci.code.Register; import jdk.vm.ci.code.RegisterValue; -import jdk.vm.ci.code.StackSlot; -import jdk.vm.ci.code.ValueUtil; import jdk.vm.ci.hotspot.HotSpotCallingConventionType; import jdk.vm.ci.hotspot.HotSpotResolvedJavaMethod; import jdk.vm.ci.meta.AllocatableValue; @@ -97,25 +93,14 @@ private AArch64HotSpotLIRGenerator getGen() { protected void emitPrologue(StructuredGraph graph) { CallingConvention incomingArguments = gen.getResult().getCallingConvention(); Value[] params = new Value[incomingArguments.getArgumentCount() + 2]; - for (int i = 0; i < incomingArguments.getArgumentCount(); i++) { - params[i] = incomingArguments.getArgument(i); - if (isStackSlot(params[i])) { - StackSlot slot = ValueUtil.asStackSlot(params[i]); - if (slot.isInCallerFrame() && !gen.getResult().getLIR().hasArgInCallerFrame()) { - gen.getResult().getLIR().setHasArgInCallerFrame(); - } - } - } + + prologAssignParams(incomingArguments, params); params[params.length - 2] = fp.asValue(LIRKind.value(AArch64Kind.QWORD)); params[params.length - 1] = lr.asValue(LIRKind.value(AArch64Kind.QWORD)); gen.emitIncomingValues(params); - for (ParameterNode param : graph.getNodes(ParameterNode.TYPE)) { - Value paramValue = params[param.index()]; - assert paramValue.getValueKind().equals(getLIRGeneratorTool().getLIRKind(param.stamp(NodeView.DEFAULT))) : paramValue.getValueKind() + " != " + param.stamp(NodeView.DEFAULT); - setResult(param, gen.emitMove(paramValue)); - } + prologSetParameterNodes(graph, params); } @Override diff --git a/compiler/src/org.graalvm.compiler.hotspot.amd64/src/org/graalvm/compiler/hotspot/amd64/AMD64HotSpotNodeLIRBuilder.java b/compiler/src/org.graalvm.compiler.hotspot.amd64/src/org/graalvm/compiler/hotspot/amd64/AMD64HotSpotNodeLIRBuilder.java index 71f4d5f1e769..fcdd9e443319 100644 --- a/compiler/src/org.graalvm.compiler.hotspot.amd64/src/org/graalvm/compiler/hotspot/amd64/AMD64HotSpotNodeLIRBuilder.java +++ b/compiler/src/org.graalvm.compiler.hotspot.amd64/src/org/graalvm/compiler/hotspot/amd64/AMD64HotSpotNodeLIRBuilder.java @@ -25,7 +25,6 @@ package org.graalvm.compiler.hotspot.amd64; import static jdk.vm.ci.amd64.AMD64.rbp; -import static jdk.vm.ci.code.ValueUtil.isStackSlot; import static org.graalvm.compiler.hotspot.HotSpotBackend.EXCEPTION_HANDLER_IN_CALLER; import org.graalvm.compiler.core.amd64.AMD64NodeLIRBuilder; @@ -50,7 +49,6 @@ import org.graalvm.compiler.nodes.FullInfopointNode; import org.graalvm.compiler.nodes.IndirectCallTargetNode; import org.graalvm.compiler.nodes.NodeView; -import org.graalvm.compiler.nodes.ParameterNode; import org.graalvm.compiler.nodes.SafepointNode; import org.graalvm.compiler.nodes.StructuredGraph; import org.graalvm.compiler.nodes.ValueNode; @@ -71,8 +69,6 @@ import jdk.vm.ci.code.CallingConvention; import jdk.vm.ci.code.Register; import jdk.vm.ci.code.RegisterValue; -import jdk.vm.ci.code.StackSlot; -import jdk.vm.ci.code.ValueUtil; import jdk.vm.ci.hotspot.HotSpotCallingConventionType; import jdk.vm.ci.hotspot.HotSpotResolvedJavaMethod; import jdk.vm.ci.meta.AllocatableValue; @@ -103,19 +99,10 @@ protected DebugInfoBuilder createDebugInfoBuilder(StructuredGraph graph, NodeVal @Override protected void emitPrologue(StructuredGraph graph) { - CallingConvention incomingArguments = gen.getResult().getCallingConvention(); - Value[] params = new Value[incomingArguments.getArgumentCount() + 1]; - for (int i = 0; i < params.length - 1; i++) { - params[i] = incomingArguments.getArgument(i); - if (isStackSlot(params[i])) { - StackSlot slot = ValueUtil.asStackSlot(params[i]); - if (slot.isInCallerFrame() && !gen.getResult().getLIR().hasArgInCallerFrame()) { - gen.getResult().getLIR().setHasArgInCallerFrame(); - } - } - } + + prologAssignParams(incomingArguments, params); params[params.length - 1] = rbp.asValue(LIRKind.value(AMD64Kind.QWORD)); gen.emitIncomingValues(params); @@ -124,12 +111,7 @@ protected void emitPrologue(StructuredGraph graph) { getGen().append(((HotSpotDebugInfoBuilder) getDebugInfoBuilder()).lockStack()); - for (ParameterNode param : graph.getNodes(ParameterNode.TYPE)) { - Value paramValue = params[param.index()]; - assert paramValue.getValueKind().equals(getLIRGeneratorTool().getLIRKind(param.stamp(NodeView.DEFAULT))) : paramValue.getValueKind() + " != " + - getLIRGeneratorTool().getLIRKind(param.stamp(NodeView.DEFAULT)) + " for " + param.stamp(NodeView.DEFAULT); - setResult(param, gen.emitMove(paramValue)); - } + prologSetParameterNodes(graph, params); } @Override diff --git a/substratevm/src/com.oracle.svm.core.graal.aarch64/src/com/oracle/svm/core/graal/aarch64/SubstrateAArch64Backend.java b/substratevm/src/com.oracle.svm.core.graal.aarch64/src/com/oracle/svm/core/graal/aarch64/SubstrateAArch64Backend.java index 98e4b4245fac..1aabe2599370 100755 --- a/substratevm/src/com.oracle.svm.core.graal.aarch64/src/com/oracle/svm/core/graal/aarch64/SubstrateAArch64Backend.java +++ b/substratevm/src/com.oracle.svm.core.graal.aarch64/src/com/oracle/svm/core/graal/aarch64/SubstrateAArch64Backend.java @@ -99,6 +99,7 @@ import org.graalvm.compiler.nodes.InvokeWithExceptionNode; import org.graalvm.compiler.nodes.LogicNode; import org.graalvm.compiler.nodes.NodeView; +import org.graalvm.compiler.nodes.ParameterNode; import org.graalvm.compiler.nodes.SafepointNode; import org.graalvm.compiler.nodes.StructuredGraph; import org.graalvm.compiler.nodes.spi.CoreProviders; @@ -120,7 +121,9 @@ import com.oracle.svm.core.deopt.Deoptimizer; import com.oracle.svm.core.graal.code.PatchConsumerFactory; import com.oracle.svm.core.graal.code.SubstrateBackend; +import com.oracle.svm.core.graal.code.SubstrateCallingConvention; import com.oracle.svm.core.graal.code.SubstrateCallingConventionKind; +import com.oracle.svm.core.graal.code.SubstrateCallingConventionType; import com.oracle.svm.core.graal.code.SubstrateCompiledCode; import com.oracle.svm.core.graal.code.SubstrateDataBuilder; import com.oracle.svm.core.graal.code.SubstrateDebugInfoBuilder; @@ -530,6 +533,35 @@ private boolean getDestroysCallerSavedRegisters(ResolvedJavaMethod targetMethod) return ((SubstrateAArch64LIRGenerator) gen).getDestroysCallerSavedRegisters(targetMethod); } + @Override + protected void prologSetParameterNodes(StructuredGraph graph, Value[] params) { + SubstrateCallingConvention convention = (SubstrateCallingConvention) gen.getResult().getCallingConvention(); + for (ParameterNode param : graph.getNodes(ParameterNode.TYPE)) { + Value inputValue = params[param.index()]; + Value paramValue = gen.emitMove(inputValue); + + /* + * In the native ABI some parameters are not extended to the equivalent Java stack + * kinds. + */ + if (inputValue.getPlatformKind().getSizeInBytes() < Integer.BYTES) { + SubstrateCallingConventionType type = (SubstrateCallingConventionType) convention.getType(); + assert !type.outgoing && type.nativeABI(); + JavaKind kind = convention.getArgumentStorageKinds()[param.index()]; + JavaKind stackKind = kind.getStackKind(); + if (kind.isUnsigned()) { + paramValue = gen.getArithmetic().emitZeroExtend(paramValue, kind.getBitCount(), stackKind.getBitCount()); + } else { + paramValue = gen.getArithmetic().emitSignExtend(paramValue, kind.getBitCount(), stackKind.getBitCount()); + } + } + + assert paramValue.getValueKind().equals(gen.getLIRKind(param.stamp(NodeView.DEFAULT))); + + setResult(param, paramValue); + } + } + /** * For invokes that have an exception handler, the register used for the incoming exception * is destroyed at the call site even when registers are caller saved. The normal object diff --git a/substratevm/src/com.oracle.svm.core.graal.aarch64/src/com/oracle/svm/core/graal/aarch64/SubstrateAArch64RegisterConfig.java b/substratevm/src/com.oracle.svm.core.graal.aarch64/src/com/oracle/svm/core/graal/aarch64/SubstrateAArch64RegisterConfig.java index 0c74890f1833..95aefcdb8e67 100755 --- a/substratevm/src/com.oracle.svm.core.graal.aarch64/src/com/oracle/svm/core/graal/aarch64/SubstrateAArch64RegisterConfig.java +++ b/substratevm/src/com.oracle.svm.core.graal.aarch64/src/com/oracle/svm/core/graal/aarch64/SubstrateAArch64RegisterConfig.java @@ -70,6 +70,7 @@ import java.util.ArrayList; import org.graalvm.compiler.core.common.NumUtil; +import org.graalvm.nativeimage.Platform; import com.oracle.svm.core.OS; import com.oracle.svm.core.ReservedRegisters; @@ -242,21 +243,31 @@ private int javaStackParameterAssignment(ValueKindFactory valueKindFactory, A } /** - * The Darwin calling convention expects arguments to be aligned to the argument kind. + * The Linux calling convention expects stack arguments to be aligned to at least 8 bytes, but + * any unused padding bits have unspecified values. + * + * For more details, see the + * AArch64 procedure call standard. + */ + private int linuxNativeStackParameterAssignment(ValueKindFactory valueKindFactory, AllocatableValue[] locations, int index, JavaKind kind, int currentStackOffset, boolean isOutgoing) { + ValueKind valueKind = valueKindFactory.getValueKind(isOutgoing ? kind.getStackKind() : kind); + int alignment = Math.max(kind.getByteCount(), target.wordSize); + locations[index] = StackSlot.get(valueKind, currentStackOffset, !isOutgoing); + return currentStackOffset + alignment; + } + + /** + * The Darwin calling convention expects stack arguments to be aligned to the argument kind. * * For more details, see https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms. */ - private int darwinNativeStackParameterAssignment(ValueKindFactory valueKindFactory, AllocatableValue[] locations, int index, JavaKind kind, int currentStackOffset, boolean isOutgoing) { - if (isOutgoing) { - int paramByteSize = kind.getByteCount(); - int alignedStackOffset = NumUtil.roundUp(currentStackOffset, paramByteSize); - locations[index] = StackSlot.get(valueKindFactory.getValueKind(kind), alignedStackOffset, false); - return alignedStackOffset + paramByteSize; - } else { - /* Native-to-Java calls follow the normal Java convention. */ - return javaStackParameterAssignment(valueKindFactory, locations, index, kind, currentStackOffset, isOutgoing); - } + private static int darwinNativeStackParameterAssignment(ValueKindFactory valueKindFactory, AllocatableValue[] locations, int index, JavaKind kind, int currentStackOffset, boolean isOutgoing) { + int paramByteSize = kind.getByteCount(); + int alignedStackOffset = NumUtil.roundUp(currentStackOffset, paramByteSize); + locations[index] = StackSlot.get(valueKindFactory.getValueKind(kind), alignedStackOffset, !isOutgoing); + return alignedStackOffset + paramByteSize; } @Override @@ -309,10 +320,28 @@ public CallingConvention getCallingConvention(Type t, JavaType returnType, JavaT } if (register != null) { - locations[i] = register.asValue(valueKindFactory.getValueKind(kind.getStackKind())); + /* + * The AArch64 procedure call standard does not require subword (i.e., boolean, + * byte, char, short) values to be extended to 32 bits. Hence, for incoming native + * calls, we can only assume the bits sizes as specified in the standard. + * + * Since within the graal compiler subwords are already extended to 32 bits, we save + * extended values in outgoing calls. + * + * Darwin deviates from the call standard and requires the caller to extend subword + * values. + */ + boolean useJavaKind = isEntryPoint && (Platform.includedIn(Platform.LINUX.class) || Platform.includedIn(Platform.WINDOWS.class)); + locations[i] = register.asValue(valueKindFactory.getValueKind(useJavaKind ? kind : kind.getStackKind())); } else { - if (type.nativeABI() && OS.DARWIN.isCurrent()) { - currentStackOffset = darwinNativeStackParameterAssignment(valueKindFactory, locations, i, kind, currentStackOffset, type.outgoing); + if (type.nativeABI()) { + if (Platform.includedIn(Platform.LINUX.class)) { + currentStackOffset = linuxNativeStackParameterAssignment(valueKindFactory, locations, i, kind, currentStackOffset, type.outgoing); + } else if (Platform.includedIn(Platform.DARWIN.class)) { + currentStackOffset = darwinNativeStackParameterAssignment(valueKindFactory, locations, i, kind, currentStackOffset, type.outgoing); + } else { + throw VMError.shouldNotReachHere(); + } } else { currentStackOffset = javaStackParameterAssignment(valueKindFactory, locations, i, kind, currentStackOffset, type.outgoing); } diff --git a/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/SubstrateAMD64Backend.java b/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/SubstrateAMD64Backend.java index 65278048bfc0..c1c70651d728 100644 --- a/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/SubstrateAMD64Backend.java +++ b/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/SubstrateAMD64Backend.java @@ -100,6 +100,7 @@ import org.graalvm.compiler.nodes.InvokeWithExceptionNode; import org.graalvm.compiler.nodes.LogicNode; import org.graalvm.compiler.nodes.NodeView; +import org.graalvm.compiler.nodes.ParameterNode; import org.graalvm.compiler.nodes.SafepointNode; import org.graalvm.compiler.nodes.StructuredGraph; import org.graalvm.compiler.nodes.ValueNode; @@ -616,6 +617,35 @@ protected DebugInfoBuilder createDebugInfoBuilder(StructuredGraph graph, NodeVal return new SubstrateDebugInfoBuilder(graph, gen.getProviders().getMetaAccessExtensionProvider(), nodeValueMap); } + @Override + protected void prologSetParameterNodes(StructuredGraph graph, Value[] params) { + SubstrateCallingConvention convention = (SubstrateCallingConvention) gen.getResult().getCallingConvention(); + for (ParameterNode param : graph.getNodes(ParameterNode.TYPE)) { + Value inputValue = params[param.index()]; + Value paramValue = gen.emitMove(inputValue); + + /* + * In the native ABI, some parameters are not extended to the equivalent Java stack + * kinds. + */ + if (inputValue.getPlatformKind().getSizeInBytes() < Integer.BYTES) { + SubstrateCallingConventionType type = (SubstrateCallingConventionType) convention.getType(); + assert !type.outgoing && type.nativeABI(); + JavaKind kind = convention.getArgumentStorageKinds()[param.index()]; + JavaKind stackKind = kind.getStackKind(); + if (kind.isUnsigned()) { + paramValue = gen.getArithmetic().emitZeroExtend(paramValue, kind.getBitCount(), stackKind.getBitCount()); + } else { + paramValue = gen.getArithmetic().emitSignExtend(paramValue, kind.getBitCount(), stackKind.getBitCount()); + } + } + + assert paramValue.getValueKind().equals(gen.getLIRKind(param.stamp(NodeView.DEFAULT))); + + setResult(param, paramValue); + } + } + @Override public Value[] visitInvokeArguments(CallingConvention invokeCc, Collection arguments) { Value[] values = super.visitInvokeArguments(invokeCc, arguments); diff --git a/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/SubstrateAMD64RegisterConfig.java b/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/SubstrateAMD64RegisterConfig.java index 8403981a48b6..05752dc791b2 100644 --- a/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/SubstrateAMD64RegisterConfig.java +++ b/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/SubstrateAMD64RegisterConfig.java @@ -272,12 +272,21 @@ public CallingConvention getCallingConvention(Type t, JavaType returnType, JavaT } } + /* + * The AMD64 ABI does not specify whether subword (i.e., boolean, byte, char, short) + * values should be extended to 32 bits. Hence, for incoming native calls, we can only + * assume the bits sizes as specified in the standard. + * + * Since within the graal compiler subwords are already extended to 32 bits, we save + * extended values in outgoing calls. Note that some compilers also expect arguments to + * be extended (https://reviews.llvm.org/rG1db979bae832563efde2523bb36ddabad43293d8). + */ + ValueKind paramValueKind = valueKindFactory.getValueKind(isEntryPoint ? kind : kind.getStackKind()); if (register != null) { - locations[i] = register.asValue(valueKindFactory.getValueKind(kind.getStackKind())); + locations[i] = register.asValue(paramValueKind); } else { - ValueKind valueKind = valueKindFactory.getValueKind(kind.getStackKind()); - locations[i] = StackSlot.get(valueKind, currentStackOffset, !type.outgoing); - currentStackOffset += Math.max(valueKind.getPlatformKind().getSizeInBytes(), target.wordSize); + locations[i] = StackSlot.get(paramValueKind, currentStackOffset, !type.outgoing); + currentStackOffset += Math.max(paramValueKind.getPlatformKind().getSizeInBytes(), target.wordSize); } }