Skip to content

Commit ac694ff

Browse files
teshullteshull
committed
[GR-36135] Set parameters for aarch64 native abi correctly.
PullRequest: graal/10827
2 parents dcfe849 + 25a9f1d commit ac694ff

File tree

8 files changed

+135
-67
lines changed

8 files changed

+135
-67
lines changed

‎compiler/src/org.graalvm.compiler.core.aarch64/src/org/graalvm/compiler/core/aarch64/AArch64NodeLIRBuilder.java‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,4 @@ protected boolean peephole(ValueNode valueNode) {
4949
public AArch64LIRGenerator getLIRGeneratorTool() {
5050
return (AArch64LIRGenerator) super.getLIRGeneratorTool();
5151
}
52-
53-
@Override
54-
protected void emitPrologue(StructuredGraph graph) {
55-
// XXX Maybe we need something like this.
56-
// getLIRGeneratorTool().emitLoadConstantTableBase();
57-
super.emitPrologue(graph);
58-
}
5952
}

‎compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/gen/NodeLIRBuilder.java‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,15 @@ protected void emitPrologue(StructuredGraph graph) {
516516
CallingConvention incomingArguments = gen.getResult().getCallingConvention();
517517

518518
Value[] params = new Value[incomingArguments.getArgumentCount()];
519-
for (int i = 0; i < params.length; i++) {
519+
prologAssignParams(incomingArguments, params);
520+
521+
gen.emitIncomingValues(params);
522+
523+
prologSetParameterNodes(graph, params);
524+
}
525+
526+
protected final void prologAssignParams(CallingConvention incomingArguments, Value[] params) {
527+
for (int i = 0; i < incomingArguments.getArgumentCount(); i++) {
520528
params[i] = incomingArguments.getArgument(i);
521529
if (ValueUtil.isStackSlot(params[i])) {
522530
StackSlot slot = ValueUtil.asStackSlot(params[i]);
@@ -525,9 +533,9 @@ protected void emitPrologue(StructuredGraph graph) {
525533
}
526534
}
527535
}
536+
}
528537

529-
gen.emitIncomingValues(params);
530-
538+
protected void prologSetParameterNodes(StructuredGraph graph, Value[] params) {
531539
for (ParameterNode param : graph.getNodes(ParameterNode.TYPE)) {
532540
Value paramValue = params[param.index()];
533541
assert paramValue.getValueKind().equals(getLIRGeneratorTool().getLIRKind(param.stamp(NodeView.DEFAULT))) : paramValue + " " +

‎compiler/src/org.graalvm.compiler.hotspot.aarch64/src/org/graalvm/compiler/hotspot/aarch64/AArch64HotSpotNodeLIRBuilder.java‎

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
package org.graalvm.compiler.hotspot.aarch64;
2626

2727
import static jdk.vm.ci.aarch64.AArch64.lr;
28-
import static jdk.vm.ci.code.ValueUtil.isStackSlot;
2928
import static jdk.vm.ci.hotspot.aarch64.AArch64HotSpotRegisterConfig.fp;
3029
import static jdk.vm.ci.hotspot.aarch64.AArch64HotSpotRegisterConfig.inlineCacheRegister;
3130
import static jdk.vm.ci.hotspot.aarch64.AArch64HotSpotRegisterConfig.metaspaceMethodRegister;
@@ -52,7 +51,6 @@
5251
import org.graalvm.compiler.nodes.FullInfopointNode;
5352
import org.graalvm.compiler.nodes.IndirectCallTargetNode;
5453
import org.graalvm.compiler.nodes.NodeView;
55-
import org.graalvm.compiler.nodes.ParameterNode;
5654
import org.graalvm.compiler.nodes.SafepointNode;
5755
import org.graalvm.compiler.nodes.StructuredGraph;
5856
import org.graalvm.compiler.nodes.ValueNode;
@@ -63,8 +61,6 @@
6361
import jdk.vm.ci.code.CallingConvention;
6462
import jdk.vm.ci.code.Register;
6563
import jdk.vm.ci.code.RegisterValue;
66-
import jdk.vm.ci.code.StackSlot;
67-
import jdk.vm.ci.code.ValueUtil;
6864
import jdk.vm.ci.hotspot.HotSpotCallingConventionType;
6965
import jdk.vm.ci.hotspot.HotSpotResolvedJavaMethod;
7066
import jdk.vm.ci.meta.AllocatableValue;
@@ -97,25 +93,14 @@ private AArch64HotSpotLIRGenerator getGen() {
9793
protected void emitPrologue(StructuredGraph graph) {
9894
CallingConvention incomingArguments = gen.getResult().getCallingConvention();
9995
Value[] params = new Value[incomingArguments.getArgumentCount() + 2];
100-
for (int i = 0; i < incomingArguments.getArgumentCount(); i++) {
101-
params[i] = incomingArguments.getArgument(i);
102-
if (isStackSlot(params[i])) {
103-
StackSlot slot = ValueUtil.asStackSlot(params[i]);
104-
if (slot.isInCallerFrame() && !gen.getResult().getLIR().hasArgInCallerFrame()) {
105-
gen.getResult().getLIR().setHasArgInCallerFrame();
106-
}
107-
}
108-
}
96+
97+
prologAssignParams(incomingArguments, params);
10998
params[params.length - 2] = fp.asValue(LIRKind.value(AArch64Kind.QWORD));
11099
params[params.length - 1] = lr.asValue(LIRKind.value(AArch64Kind.QWORD));
111100

112101
gen.emitIncomingValues(params);
113102

114-
for (ParameterNode param : graph.getNodes(ParameterNode.TYPE)) {
115-
Value paramValue = params[param.index()];
116-
assert paramValue.getValueKind().equals(getLIRGeneratorTool().getLIRKind(param.stamp(NodeView.DEFAULT))) : paramValue.getValueKind() + " != " + param.stamp(NodeView.DEFAULT);
117-
setResult(param, gen.emitMove(paramValue));
118-
}
103+
prologSetParameterNodes(graph, params);
119104
}
120105

121106
@Override

‎compiler/src/org.graalvm.compiler.hotspot.amd64/src/org/graalvm/compiler/hotspot/amd64/AMD64HotSpotNodeLIRBuilder.java‎

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
package org.graalvm.compiler.hotspot.amd64;
2626

2727
import static jdk.vm.ci.amd64.AMD64.rbp;
28-
import static jdk.vm.ci.code.ValueUtil.isStackSlot;
2928
import static org.graalvm.compiler.hotspot.HotSpotBackend.EXCEPTION_HANDLER_IN_CALLER;
3029

3130
import org.graalvm.compiler.core.amd64.AMD64NodeLIRBuilder;
@@ -50,7 +49,6 @@
5049
import org.graalvm.compiler.nodes.FullInfopointNode;
5150
import org.graalvm.compiler.nodes.IndirectCallTargetNode;
5251
import org.graalvm.compiler.nodes.NodeView;
53-
import org.graalvm.compiler.nodes.ParameterNode;
5452
import org.graalvm.compiler.nodes.SafepointNode;
5553
import org.graalvm.compiler.nodes.StructuredGraph;
5654
import org.graalvm.compiler.nodes.ValueNode;
@@ -71,8 +69,6 @@
7169
import jdk.vm.ci.code.CallingConvention;
7270
import jdk.vm.ci.code.Register;
7371
import jdk.vm.ci.code.RegisterValue;
74-
import jdk.vm.ci.code.StackSlot;
75-
import jdk.vm.ci.code.ValueUtil;
7672
import jdk.vm.ci.hotspot.HotSpotCallingConventionType;
7773
import jdk.vm.ci.hotspot.HotSpotResolvedJavaMethod;
7874
import jdk.vm.ci.meta.AllocatableValue;
@@ -103,19 +99,10 @@ protected DebugInfoBuilder createDebugInfoBuilder(StructuredGraph graph, NodeVal
10399

104100
@Override
105101
protected void emitPrologue(StructuredGraph graph) {
106-
107102
CallingConvention incomingArguments = gen.getResult().getCallingConvention();
108-
109103
Value[] params = new Value[incomingArguments.getArgumentCount() + 1];
110-
for (int i = 0; i < params.length - 1; i++) {
111-
params[i] = incomingArguments.getArgument(i);
112-
if (isStackSlot(params[i])) {
113-
StackSlot slot = ValueUtil.asStackSlot(params[i]);
114-
if (slot.isInCallerFrame() && !gen.getResult().getLIR().hasArgInCallerFrame()) {
115-
gen.getResult().getLIR().setHasArgInCallerFrame();
116-
}
117-
}
118-
}
104+
105+
prologAssignParams(incomingArguments, params);
119106
params[params.length - 1] = rbp.asValue(LIRKind.value(AMD64Kind.QWORD));
120107

121108
gen.emitIncomingValues(params);
@@ -124,12 +111,7 @@ protected void emitPrologue(StructuredGraph graph) {
124111

125112
getGen().append(((HotSpotDebugInfoBuilder) getDebugInfoBuilder()).lockStack());
126113

127-
for (ParameterNode param : graph.getNodes(ParameterNode.TYPE)) {
128-
Value paramValue = params[param.index()];
129-
assert paramValue.getValueKind().equals(getLIRGeneratorTool().getLIRKind(param.stamp(NodeView.DEFAULT))) : paramValue.getValueKind() + " != " +
130-
getLIRGeneratorTool().getLIRKind(param.stamp(NodeView.DEFAULT)) + " for " + param.stamp(NodeView.DEFAULT);
131-
setResult(param, gen.emitMove(paramValue));
132-
}
114+
prologSetParameterNodes(graph, params);
133115
}
134116

135117
@Override

‎substratevm/src/com.oracle.svm.core.graal.aarch64/src/com/oracle/svm/core/graal/aarch64/SubstrateAArch64Backend.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@
9999
import org.graalvm.compiler.nodes.InvokeWithExceptionNode;
100100
import org.graalvm.compiler.nodes.LogicNode;
101101
import org.graalvm.compiler.nodes.NodeView;
102+
import org.graalvm.compiler.nodes.ParameterNode;
102103
import org.graalvm.compiler.nodes.SafepointNode;
103104
import org.graalvm.compiler.nodes.StructuredGraph;
104105
import org.graalvm.compiler.nodes.spi.CoreProviders;
@@ -120,7 +121,9 @@
120121
import com.oracle.svm.core.deopt.Deoptimizer;
121122
import com.oracle.svm.core.graal.code.PatchConsumerFactory;
122123
import com.oracle.svm.core.graal.code.SubstrateBackend;
124+
import com.oracle.svm.core.graal.code.SubstrateCallingConvention;
123125
import com.oracle.svm.core.graal.code.SubstrateCallingConventionKind;
126+
import com.oracle.svm.core.graal.code.SubstrateCallingConventionType;
124127
import com.oracle.svm.core.graal.code.SubstrateCompiledCode;
125128
import com.oracle.svm.core.graal.code.SubstrateDataBuilder;
126129
import com.oracle.svm.core.graal.code.SubstrateDebugInfoBuilder;
@@ -530,6 +533,35 @@ private boolean getDestroysCallerSavedRegisters(ResolvedJavaMethod targetMethod)
530533
return ((SubstrateAArch64LIRGenerator) gen).getDestroysCallerSavedRegisters(targetMethod);
531534
}
532535

536+
@Override
537+
protected void prologSetParameterNodes(StructuredGraph graph, Value[] params) {
538+
SubstrateCallingConvention convention = (SubstrateCallingConvention) gen.getResult().getCallingConvention();
539+
for (ParameterNode param : graph.getNodes(ParameterNode.TYPE)) {
540+
Value inputValue = params[param.index()];
541+
Value paramValue = gen.emitMove(inputValue);
542+
543+
/*
544+
* In the native ABI some parameters are not extended to the equivalent Java stack
545+
* kinds.
546+
*/
547+
if (inputValue.getPlatformKind().getSizeInBytes() < Integer.BYTES) {
548+
SubstrateCallingConventionType type = (SubstrateCallingConventionType) convention.getType();
549+
assert !type.outgoing && type.nativeABI();
550+
JavaKind kind = convention.getArgumentStorageKinds()[param.index()];
551+
JavaKind stackKind = kind.getStackKind();
552+
if (kind.isUnsigned()) {
553+
paramValue = gen.getArithmetic().emitZeroExtend(paramValue, kind.getBitCount(), stackKind.getBitCount());
554+
} else {
555+
paramValue = gen.getArithmetic().emitSignExtend(paramValue, kind.getBitCount(), stackKind.getBitCount());
556+
}
557+
}
558+
559+
assert paramValue.getValueKind().equals(gen.getLIRKind(param.stamp(NodeView.DEFAULT)));
560+
561+
setResult(param, paramValue);
562+
}
563+
}
564+
533565
/**
534566
* For invokes that have an exception handler, the register used for the incoming exception
535567
* is destroyed at the call site even when registers are caller saved. The normal object

‎substratevm/src/com.oracle.svm.core.graal.aarch64/src/com/oracle/svm/core/graal/aarch64/SubstrateAArch64RegisterConfig.java‎

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
import java.util.ArrayList;
7171

7272
import org.graalvm.compiler.core.common.NumUtil;
73+
import org.graalvm.nativeimage.Platform;
7374

7475
import com.oracle.svm.core.OS;
7576
import com.oracle.svm.core.ReservedRegisters;
@@ -242,21 +243,31 @@ private int javaStackParameterAssignment(ValueKindFactory<?> valueKindFactory, A
242243
}
243244

244245
/**
245-
* The Darwin calling convention expects arguments to be aligned to the argument kind.
246+
* The Linux calling convention expects stack arguments to be aligned to at least 8 bytes, but
247+
* any unused padding bits have unspecified values.
248+
*
249+
* For more details, see <a
250+
* href=https://github.com/ARM-software/abi-aa/blob/d6e9abbc5e9cdcaa0467d8187eec0049b44044c4/aapcs64/aapcs64.rst#parameter-passing-rules>the
251+
* AArch64 procedure call standard</a>.
252+
*/
253+
private int linuxNativeStackParameterAssignment(ValueKindFactory<?> valueKindFactory, AllocatableValue[] locations, int index, JavaKind kind, int currentStackOffset, boolean isOutgoing) {
254+
ValueKind<?> valueKind = valueKindFactory.getValueKind(isOutgoing ? kind.getStackKind() : kind);
255+
int alignment = Math.max(kind.getByteCount(), target.wordSize);
256+
locations[index] = StackSlot.get(valueKind, currentStackOffset, !isOutgoing);
257+
return currentStackOffset + alignment;
258+
}
259+
260+
/**
261+
* The Darwin calling convention expects stack arguments to be aligned to the argument kind.
246262
*
247263
* For more details, see <a
248264
* href=https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms>https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms</a>.
249265
*/
250-
private int darwinNativeStackParameterAssignment(ValueKindFactory<?> valueKindFactory, AllocatableValue[] locations, int index, JavaKind kind, int currentStackOffset, boolean isOutgoing) {
251-
if (isOutgoing) {
252-
int paramByteSize = kind.getByteCount();
253-
int alignedStackOffset = NumUtil.roundUp(currentStackOffset, paramByteSize);
254-
locations[index] = StackSlot.get(valueKindFactory.getValueKind(kind), alignedStackOffset, false);
255-
return alignedStackOffset + paramByteSize;
256-
} else {
257-
/* Native-to-Java calls follow the normal Java convention. */
258-
return javaStackParameterAssignment(valueKindFactory, locations, index, kind, currentStackOffset, isOutgoing);
259-
}
266+
private static int darwinNativeStackParameterAssignment(ValueKindFactory<?> valueKindFactory, AllocatableValue[] locations, int index, JavaKind kind, int currentStackOffset, boolean isOutgoing) {
267+
int paramByteSize = kind.getByteCount();
268+
int alignedStackOffset = NumUtil.roundUp(currentStackOffset, paramByteSize);
269+
locations[index] = StackSlot.get(valueKindFactory.getValueKind(kind), alignedStackOffset, !isOutgoing);
270+
return alignedStackOffset + paramByteSize;
260271
}
261272

262273
@Override
@@ -309,10 +320,28 @@ public CallingConvention getCallingConvention(Type t, JavaType returnType, JavaT
309320

310321
}
311322
if (register != null) {
312-
locations[i] = register.asValue(valueKindFactory.getValueKind(kind.getStackKind()));
323+
/*
324+
* The AArch64 procedure call standard does not require subword (i.e., boolean,
325+
* byte, char, short) values to be extended to 32 bits. Hence, for incoming native
326+
* calls, we can only assume the bits sizes as specified in the standard.
327+
*
328+
* Since within the graal compiler subwords are already extended to 32 bits, we save
329+
* extended values in outgoing calls.
330+
*
331+
* Darwin deviates from the call standard and requires the caller to extend subword
332+
* values.
333+
*/
334+
boolean useJavaKind = isEntryPoint && (Platform.includedIn(Platform.LINUX.class) || Platform.includedIn(Platform.WINDOWS.class));
335+
locations[i] = register.asValue(valueKindFactory.getValueKind(useJavaKind ? kind : kind.getStackKind()));
313336
} else {
314-
if (type.nativeABI() && OS.DARWIN.isCurrent()) {
315-
currentStackOffset = darwinNativeStackParameterAssignment(valueKindFactory, locations, i, kind, currentStackOffset, type.outgoing);
337+
if (type.nativeABI()) {
338+
if (Platform.includedIn(Platform.LINUX.class)) {
339+
currentStackOffset = linuxNativeStackParameterAssignment(valueKindFactory, locations, i, kind, currentStackOffset, type.outgoing);
340+
} else if (Platform.includedIn(Platform.DARWIN.class)) {
341+
currentStackOffset = darwinNativeStackParameterAssignment(valueKindFactory, locations, i, kind, currentStackOffset, type.outgoing);
342+
} else {
343+
throw VMError.shouldNotReachHere();
344+
}
316345
} else {
317346
currentStackOffset = javaStackParameterAssignment(valueKindFactory, locations, i, kind, currentStackOffset, type.outgoing);
318347
}

‎substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/SubstrateAMD64Backend.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@
100100
import org.graalvm.compiler.nodes.InvokeWithExceptionNode;
101101
import org.graalvm.compiler.nodes.LogicNode;
102102
import org.graalvm.compiler.nodes.NodeView;
103+
import org.graalvm.compiler.nodes.ParameterNode;
103104
import org.graalvm.compiler.nodes.SafepointNode;
104105
import org.graalvm.compiler.nodes.StructuredGraph;
105106
import org.graalvm.compiler.nodes.ValueNode;
@@ -616,6 +617,35 @@ protected DebugInfoBuilder createDebugInfoBuilder(StructuredGraph graph, NodeVal
616617
return new SubstrateDebugInfoBuilder(graph, gen.getProviders().getMetaAccessExtensionProvider(), nodeValueMap);
617618
}
618619

620+
@Override
621+
protected void prologSetParameterNodes(StructuredGraph graph, Value[] params) {
622+
SubstrateCallingConvention convention = (SubstrateCallingConvention) gen.getResult().getCallingConvention();
623+
for (ParameterNode param : graph.getNodes(ParameterNode.TYPE)) {
624+
Value inputValue = params[param.index()];
625+
Value paramValue = gen.emitMove(inputValue);
626+
627+
/*
628+
* In the native ABI, some parameters are not extended to the equivalent Java stack
629+
* kinds.
630+
*/
631+
if (inputValue.getPlatformKind().getSizeInBytes() < Integer.BYTES) {
632+
SubstrateCallingConventionType type = (SubstrateCallingConventionType) convention.getType();
633+
assert !type.outgoing && type.nativeABI();
634+
JavaKind kind = convention.getArgumentStorageKinds()[param.index()];
635+
JavaKind stackKind = kind.getStackKind();
636+
if (kind.isUnsigned()) {
637+
paramValue = gen.getArithmetic().emitZeroExtend(paramValue, kind.getBitCount(), stackKind.getBitCount());
638+
} else {
639+
paramValue = gen.getArithmetic().emitSignExtend(paramValue, kind.getBitCount(), stackKind.getBitCount());
640+
}
641+
}
642+
643+
assert paramValue.getValueKind().equals(gen.getLIRKind(param.stamp(NodeView.DEFAULT)));
644+
645+
setResult(param, paramValue);
646+
}
647+
}
648+
619649
@Override
620650
public Value[] visitInvokeArguments(CallingConvention invokeCc, Collection<ValueNode> arguments) {
621651
Value[] values = super.visitInvokeArguments(invokeCc, arguments);

‎substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/SubstrateAMD64RegisterConfig.java‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -272,12 +272,21 @@ public CallingConvention getCallingConvention(Type t, JavaType returnType, JavaT
272272
}
273273
}
274274

275+
/*
276+
* The AMD64 ABI does not specify whether subword (i.e., boolean, byte, char, short)
277+
* values should be extended to 32 bits. Hence, for incoming native calls, we can only
278+
* assume the bits sizes as specified in the standard.
279+
*
280+
* Since within the graal compiler subwords are already extended to 32 bits, we save
281+
* extended values in outgoing calls. Note that some compilers also expect arguments to
282+
* be extended (https://reviews.llvm.org/rG1db979bae832563efde2523bb36ddabad43293d8).
283+
*/
284+
ValueKind<?> paramValueKind = valueKindFactory.getValueKind(isEntryPoint ? kind : kind.getStackKind());
275285
if (register != null) {
276-
locations[i] = register.asValue(valueKindFactory.getValueKind(kind.getStackKind()));
286+
locations[i] = register.asValue(paramValueKind);
277287
} else {
278-
ValueKind<?> valueKind = valueKindFactory.getValueKind(kind.getStackKind());
279-
locations[i] = StackSlot.get(valueKind, currentStackOffset, !type.outgoing);
280-
currentStackOffset += Math.max(valueKind.getPlatformKind().getSizeInBytes(), target.wordSize);
288+
locations[i] = StackSlot.get(paramValueKind, currentStackOffset, !type.outgoing);
289+
currentStackOffset += Math.max(paramValueKind.getPlatformKind().getSizeInBytes(), target.wordSize);
281290
}
282291
}
283292

0 commit comments

Comments
 (0)