Skip to content

Commit c97d056

Browse files
pejovicapejovica
committed
[GR-51560] Simplify rbp handling in prologues/epilogues.
PullRequest: graal/17359
2 parents 888fcaa + 0f2e13b commit c97d056

File tree

9 files changed

+121
-55
lines changed

9 files changed

+121
-55
lines changed

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,12 @@ public static void createAndRegister() {
101101
calleeSavedRegisters.remove(SubstrateControlFlowIntegrity.singleton().getCFITargetRegister());
102102
}
103103

104+
/*
105+
* Handling of the rbp register is always done in the method prologue, so we remove it here
106+
* and record whether it is callee saved.
107+
*/
108+
boolean rbpCalleeSaved = calleeSavedRegisters.remove(AMD64.rbp);
109+
104110
/*
105111
* Reverse list so that CPU registers are spilled close to the beginning of the frame, i.e.,
106112
* with a closer-to-0 negative reference map index in the caller frame. That makes the
@@ -151,9 +157,18 @@ public static void createAndRegister() {
151157
int calleeSavedRegistersSizeInBytes = offset;
152158

153159
int saveAreaOffsetInFrame = -(FrameAccess.returnAddressSize() +
154-
(SubstrateOptions.PreserveFramePointer.getValue() ? FrameAccess.wordSize() : 0) +
160+
FrameAccess.wordSize() + /* Space is always reserved for rbp. */
155161
calleeSavedRegistersSizeInBytes);
156162

163+
if (rbpCalleeSaved) {
164+
/*
165+
* When rbp is callee saved, it is pushed onto the stack in the method prologue right
166+
* after the return address, i.e., at the top of the callee save area, so we are just
167+
* referencing that location here.
168+
*/
169+
calleeSavedRegisterOffsets.put(AMD64.rbp, calleeSavedRegistersSizeInBytes);
170+
}
171+
157172
ImageSingletons.add(CalleeSavedRegisters.class,
158173
new AMD64CalleeSavedRegisters(frameRegister, calleeSavedRegisters, calleeSavedXMMRegisters, calleeSavedMaskRegisters, calleeSavedRegisterOffsets,
159174
calleeSavedRegistersSizeInBytes, saveAreaOffsetInFrame, isRuntimeCompilationEnabled));
@@ -537,7 +552,7 @@ private static void dumpReg(Log log, String label, Pointer callerSP, int offsetI
537552
@Fold
538553
static int offsetInFrameOrNull(Register register) {
539554
AMD64CalleeSavedRegisters that = AMD64CalleeSavedRegisters.singleton();
540-
if (that.calleeSavedRegisters.contains(register)) {
555+
if (that.calleeSaveable(register)) {
541556
return that.getOffsetInFrame(register);
542557
} else {
543558
return 0;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public void emitCode(CompilationResultBuilder crb, AMD64MacroAssembler masm) {
9797
* of the new stack pointer.
9898
*/
9999
int calleeFrameSize = FrameAccess.returnAddressSize();
100-
if (SubstrateOptions.PreserveFramePointer.getValue()) {
100+
if (fromMethodWithCalleeSavedRegisters || SubstrateOptions.PreserveFramePointer.getValue()) {
101101
calleeFrameSize += FrameAccess.wordSize();
102102
}
103103
if (fromMethodWithCalleeSavedRegisters) {
@@ -117,7 +117,7 @@ public void emitCode(CompilationResultBuilder crb, AMD64MacroAssembler masm) {
117117
AMD64CalleeSavedRegisters.singleton().emitRestore(masm, calleeFrameSize, asRegister(result), crb);
118118
masm.incrementq(AMD64.rsp, CalleeSavedRegisters.singleton().getSaveAreaSize());
119119
}
120-
if (SubstrateOptions.PreserveFramePointer.getValue()) {
120+
if (fromMethodWithCalleeSavedRegisters || SubstrateOptions.PreserveFramePointer.getValue()) {
121121
masm.pop(AMD64.rbp);
122122
}
123123

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

Lines changed: 71 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@
188188
import jdk.vm.ci.code.CompilationRequest;
189189
import jdk.vm.ci.code.CompiledCode;
190190
import jdk.vm.ci.code.Register;
191+
import jdk.vm.ci.code.RegisterAttributes;
191192
import jdk.vm.ci.code.RegisterConfig;
192193
import jdk.vm.ci.code.RegisterValue;
193194
import jdk.vm.ci.code.StackSlot;
@@ -1097,6 +1098,41 @@ private static ForeignCallDescriptor chooseCPUFeatureVariant(ForeignCallDescript
10971098
}
10981099
}
10991100

1101+
/**
1102+
* Generates a method's prologue and epilogue.
1103+
* <p>
1104+
* Depending on the {@link SubstrateAMD64FrameMap frame map} properties and whether the rbp
1105+
* register is saved by the caller or the callee, we use different forms of prologue and
1106+
* epilogue.
1107+
*
1108+
* <pre>
1109+
*
1110+
* | preserveFramePointer |
1111+
* +----------------+----------------+
1112+
* | false | true |
1113+
* --------+----------------+----------------+
1114+
* | ; prologue | ; prologue |
1115+
* | sub rsp, #fs | push rbp |
1116+
* | | mov rbp, rsp |
1117+
* rbp is | | sub rsp, #fs |
1118+
* caller | | |
1119+
* saved | ; epilogue | ; epilogue |
1120+
* | add rsp, #fs | add rsp, #fs |
1121+
* | ret | pop rbp |
1122+
* | | ret |
1123+
* --------+----------------+----------------+
1124+
* | ; prologue | ; prologue |
1125+
* | push rbp | push rbp |
1126+
* | sub rsp, #fs | mov rbp, rsp |
1127+
* rbp is | | sub rsp, #fs |
1128+
* callee | | |
1129+
* saved | ; epilogue | ; epilogue |
1130+
* | add rsp, #fs | add rsp, #fs |
1131+
* | pop rbp | pop rbp |
1132+
* | ret | ret |
1133+
* --------+----------------+----------------+
1134+
* </pre>
1135+
*/
11001136
protected static class SubstrateAMD64FrameContext implements FrameContext {
11011137

11021138
protected final SharedMethod method;
@@ -1132,36 +1168,38 @@ protected final void reserveStackFrame(CompilationResultBuilder crb, AMD64MacroA
11321168
}
11331169

11341170
protected void maybePushBasePointer(CompilationResultBuilder crb, AMD64MacroAssembler asm) {
1135-
if (((SubstrateAMD64RegisterConfig) crb.frameMap.getRegisterConfig()).shouldUseBasePointer()) {
1171+
SubstrateAMD64FrameMap frameMap = (SubstrateAMD64FrameMap) crb.frameMap;
1172+
if (frameMap.preserveFramePointer()) {
11361173
/*
11371174
* Note that we never use the `enter` instruction so that we have a predictable code
11381175
* pattern at each method prologue. And `enter` seems to be slower than the explicit
11391176
* code.
11401177
*/
11411178
asm.push(rbp);
11421179
asm.movq(rbp, rsp);
1180+
} else if (isCalleeSaved(rbp, frameMap.getRegisterConfig(), method)) {
1181+
asm.push(rbp);
11431182
}
11441183
}
11451184

11461185
@Override
11471186
public void leave(CompilationResultBuilder crb) {
11481187
AMD64MacroAssembler asm = (AMD64MacroAssembler) crb.asm;
1188+
SubstrateAMD64FrameMap frameMap = (SubstrateAMD64FrameMap) crb.frameMap;
11491189
crb.recordMark(SubstrateMarkId.EPILOGUE_START);
11501190

11511191
if (method.hasCalleeSavedRegisters()) {
11521192
JavaKind returnKind = method.getSignature().getReturnKind();
11531193
Register returnRegister = null;
11541194
if (returnKind != JavaKind.Void) {
1155-
returnRegister = crb.frameMap.getRegisterConfig().getReturnRegister(returnKind);
1195+
returnRegister = frameMap.getRegisterConfig().getReturnRegister(returnKind);
11561196
}
1157-
AMD64CalleeSavedRegisters.singleton().emitRestore((AMD64MacroAssembler) crb.asm, crb.frameMap.totalFrameSize(), returnRegister, crb);
1197+
AMD64CalleeSavedRegisters.singleton().emitRestore(asm, frameMap.totalFrameSize(), returnRegister, crb);
11581198
}
11591199

1160-
if (((SubstrateAMD64RegisterConfig) crb.frameMap.getRegisterConfig()).shouldUseBasePointer()) {
1161-
asm.movq(rsp, rbp);
1200+
asm.incrementq(rsp, frameMap.frameSize());
1201+
if (frameMap.preserveFramePointer() || isCalleeSaved(rbp, frameMap.getRegisterConfig(), method)) {
11621202
asm.pop(rbp);
1163-
} else {
1164-
asm.incrementq(rsp, crb.frameMap.frameSize());
11651203
}
11661204

11671205
crb.recordMark(SubstrateMarkId.EPILOGUE_INCD_RSP);
@@ -1412,11 +1450,31 @@ public void emitCode(CompilationResultBuilder crb, AMD64MacroAssembler masm) {
14121450
}
14131451
}
14141452

1415-
private FrameMapBuilder newFrameMapBuilder(RegisterConfig registerConfig) {
1416-
RegisterConfig registerConfigNonNull = registerConfig == null ? getCodeCache().getRegisterConfig() : registerConfig;
1417-
FrameMap frameMap = new AMD64FrameMap(getProviders().getCodeCache(), registerConfigNonNull, new SubstrateReferenceMapBuilderFactory(),
1418-
((SubstrateAMD64RegisterConfig) registerConfigNonNull).shouldUseBasePointer());
1419-
return new AMD64FrameMapBuilder(frameMap, getCodeCache(), registerConfigNonNull);
1453+
private FrameMapBuilder newFrameMapBuilder(RegisterConfig registerConfig, SharedMethod method) {
1454+
FrameMap frameMap = new SubstrateAMD64FrameMap(getCodeCache(), (SubstrateAMD64RegisterConfig) registerConfig, new SubstrateReferenceMapBuilderFactory(), method);
1455+
return new AMD64FrameMapBuilder(frameMap, getCodeCache(), registerConfig);
1456+
}
1457+
1458+
/**
1459+
* AMD64 Substrate VM specific frame map.
1460+
* <p>
1461+
* The layout is basically the same as {@link AMD64FrameMap} except that space for rbp is also
1462+
* reserved when rbp is callee saved, not just if {@link #preserveFramePointer} is true.
1463+
*/
1464+
static class SubstrateAMD64FrameMap extends AMD64FrameMap {
1465+
SubstrateAMD64FrameMap(CodeCacheProvider codeCache, SubstrateAMD64RegisterConfig registerConfig, ReferenceMapBuilderFactory referenceMapFactory, SharedMethod method) {
1466+
super(codeCache, registerConfig, referenceMapFactory, registerConfig.shouldUseBasePointer());
1467+
if (!preserveFramePointer() && isCalleeSaved(rbp, registerConfig, method)) {
1468+
assert initialSpillSize == returnAddressSize() && spillSize == initialSpillSize : "rbp must be right after the return address";
1469+
initialSpillSize += getTarget().wordSize;
1470+
spillSize += getTarget().wordSize;
1471+
}
1472+
}
1473+
}
1474+
1475+
private static boolean isCalleeSaved(Register register, RegisterConfig config, SharedMethod method) {
1476+
RegisterAttributes registerAttributes = config.getAttributesMap()[register.number];
1477+
return registerAttributes.isCalleeSave() || registerAttributes.isAllocatable() && method.hasCalleeSavedRegisters();
14201478
}
14211479

14221480
@Override
@@ -1425,7 +1483,7 @@ public LIRGenerationResult newLIRGenerationResult(CompilationIdentifier compilat
14251483
SubstrateCallingConventionKind ccKind = method.getCallingConventionKind();
14261484
SubstrateCallingConventionType ccType = ccKind.isCustom() ? method.getCustomCallingConventionType() : ccKind.toType(false);
14271485
CallingConvention callingConvention = CodeUtil.getCallingConvention(getCodeCache(), ccType, method, this);
1428-
return new SubstrateLIRGenerationResult(compilationId, lir, newFrameMapBuilder(registerAllocationConfig.getRegisterConfig()), callingConvention, registerAllocationConfig, method);
1486+
return new SubstrateLIRGenerationResult(compilationId, lir, newFrameMapBuilder(registerAllocationConfig.getRegisterConfig(), method), callingConvention, registerAllocationConfig, method);
14291487
}
14301488

14311489
protected AMD64ArithmeticLIRGenerator createArithmeticLIRGen(RegisterValue nullRegisterValue) {

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/FrameAccess.java‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,6 @@ public static int returnAddressSize() {
6868
@Fold
6969
public abstract int stackPointerAdjustmentOnCall();
7070

71-
/**
72-
* Returns the size in bytes of the saved base pointer in the stack frame. The saved base
73-
* pointer must be located immediately after the return address (if this is not the case in a
74-
* new architecture, bigger modifications to code like the Deoptimizer is required).
75-
*/
76-
public abstract int savedBasePointerSize();
77-
7871
@Fold
7972
public static int wordSize() {
8073
return ConfigurationValues.getTarget().arch.getWordSize();

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/aarch64/AArch64FrameAccess.java‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
*/
2525
package com.oracle.svm.core.aarch64;
2626

27-
import jdk.graal.compiler.api.replacements.Fold;
2827
import org.graalvm.nativeimage.Platform;
2928
import org.graalvm.nativeimage.Platforms;
3029
import org.graalvm.nativeimage.c.function.CodePointer;
@@ -36,6 +35,8 @@
3635
import com.oracle.svm.core.feature.AutomaticallyRegisteredImageSingleton;
3736
import com.oracle.svm.core.graal.nodes.aarch64.AArch64XPACNode;
3837

38+
import jdk.graal.compiler.api.replacements.Fold;
39+
3940
@AutomaticallyRegisteredImageSingleton(FrameAccess.class)
4041
@Platforms(Platform.AARCH64.class)
4142
public class AArch64FrameAccess extends FrameAccess {
@@ -63,13 +64,6 @@ public Pointer getReturnAddressLocation(Pointer sourceSp) {
6364
return sourceSp.subtract(returnAddressSize());
6465
}
6566

66-
@Fold
67-
@Override
68-
public int savedBasePointerSize() {
69-
// The base pointer is always saved with stp instruction on method entry
70-
return wordSize();
71-
}
72-
7367
@Override
7468
@Fold
7569
public int stackPointerAdjustmentOnCall() {

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/amd64/AMD64FrameAccess.java‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,17 @@
2424
*/
2525
package com.oracle.svm.core.amd64;
2626

27-
import jdk.graal.compiler.api.replacements.Fold;
2827
import org.graalvm.nativeimage.Platform.AMD64;
2928
import org.graalvm.nativeimage.Platforms;
3029
import org.graalvm.nativeimage.c.function.CodePointer;
3130
import org.graalvm.word.Pointer;
3231

3332
import com.oracle.svm.core.FrameAccess;
34-
import com.oracle.svm.core.SubstrateOptions;
3533
import com.oracle.svm.core.Uninterruptible;
3634
import com.oracle.svm.core.feature.AutomaticallyRegisteredImageSingleton;
3735

36+
import jdk.graal.compiler.api.replacements.Fold;
37+
3838
@AutomaticallyRegisteredImageSingleton(FrameAccess.class)
3939
@Platforms(AMD64.class)
4040
public final class AMD64FrameAccess extends FrameAccess {
@@ -58,16 +58,6 @@ public Pointer getReturnAddressLocation(Pointer sourceSp) {
5858
return sourceSp.subtract(returnAddressSize());
5959
}
6060

61-
@Fold
62-
@Override
63-
public int savedBasePointerSize() {
64-
if (SubstrateOptions.PreserveFramePointer.getValue()) {
65-
return wordSize();
66-
} else {
67-
return 0;
68-
}
69-
}
70-
7161
@Override
7262
@Fold
7363
public int stackPointerAdjustmentOnCall() {

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/deopt/Deoptimizer.java‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@
3333
import java.util.ArrayList;
3434

3535
import org.graalvm.nativeimage.CurrentIsolate;
36+
import org.graalvm.nativeimage.ImageSingletons;
3637
import org.graalvm.nativeimage.IsolateThread;
38+
import org.graalvm.nativeimage.Platform;
3739
import org.graalvm.nativeimage.c.function.CodePointer;
3840
import org.graalvm.word.Pointer;
3941
import org.graalvm.word.PointerBase;
@@ -82,6 +84,7 @@
8284
import com.oracle.svm.core.util.TimeUtils;
8385
import com.oracle.svm.core.util.VMError;
8486

87+
import jdk.graal.compiler.api.replacements.Fold;
8588
import jdk.graal.compiler.core.common.NumUtil;
8689
import jdk.graal.compiler.core.common.util.TypeConversion;
8790
import jdk.graal.compiler.options.Option;
@@ -612,7 +615,7 @@ private static UnsignedWord rewriteStackStub(Pointer newSp, UnsignedWord gpRetur
612615
* deoptimizeInRange(). So when this method returns we are inside the caller of
613616
* deoptimizeInRange().
614617
*/
615-
Pointer bottomSp = newSp.subtract(FrameAccess.returnAddressSize() + FrameAccess.singleton().savedBasePointerSize());
618+
Pointer bottomSp = newSp.subtract(FrameAccess.returnAddressSize() + savedBasePointerSize());
616619
frame.getTargetContent().copyToPointer(bottomSp);
617620

618621
if (DeoptimizationCounters.Options.ProfileDeoptimization.getValue()) {
@@ -622,6 +625,25 @@ private static UnsignedWord rewriteStackStub(Pointer newSp, UnsignedWord gpRetur
622625
return gpReturnValue;
623626
}
624627

628+
/**
629+
* Returns the size in bytes of the saved base pointer in the stack frame. The saved base
630+
* pointer must be located immediately after the return address (if this is not the case in a
631+
* new architecture, bigger modifications to the Deoptimizer code are required).
632+
*/
633+
@Fold
634+
static int savedBasePointerSize() {
635+
if (Platform.includedIn(Platform.AMD64.class)) {
636+
return SubstrateOptions.PreserveFramePointer.getValue() ? FrameAccess.wordSize() : 0;
637+
} else if (Platform.includedIn(Platform.AARCH64.class)) {
638+
// The base pointer is always saved with stp instruction on method entry
639+
return FrameAccess.wordSize();
640+
} else if (Platform.includedIn(Platform.RISCV64.class)) {
641+
// The base pointer is always pushed on the stack on method entry
642+
return FrameAccess.wordSize();
643+
}
644+
throw VMError.shouldNotReachHere("Unexpected platform: " + ImageSingletons.lookup(Platform.class));
645+
}
646+
625647
/**
626648
* Reads the value of a local variable in the given frame. If the local variable is a virtual
627649
* object, the object (and all other objects reachable from it) are materialized.
@@ -800,7 +822,7 @@ public static void logRecentDeoptimizationEvents(Log log) {
800822
*/
801823
private VirtualFrame constructTargetFrame(CodeInfoQueryResult targetInfo, FrameInfoQueryResult sourceFrame) {
802824
FrameInfoQueryResult targetFrame = targetInfo.getFrameInfo();
803-
int savedBasePointerSize = FrameAccess.singleton().savedBasePointerSize();
825+
int savedBasePointerSize = savedBasePointerSize();
804826
int targetFrameSize = NumUtil.safeToInt(targetInfo.getTotalFrameSize()) - FrameAccess.returnAddressSize() - savedBasePointerSize;
805827
VirtualFrame result = new VirtualFrame(targetFrame);
806828

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/sampler/AbstractJfrExecutionSampler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ private static void doUninterruptibleStackWalk(SamplerSampleWriterData data, Poi
332332
* We are in the prologue or epilogue. Frame pointer and return address are on top
333333
* of the stack.
334334
*/
335-
callerSP = sp.add(FrameAccess.wordSize()).add(FrameAccess.singleton().savedBasePointerSize());
335+
callerSP = sp.add(FrameAccess.wordSize()).add(FrameAccess.wordSize());
336336
}
337337
} else {
338338
/* We are in the prologue or epilogue. Return address is at the top of the stack. */

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/riscv64/RISCV64FrameAccess.java‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
*/
2525
package com.oracle.svm.core.riscv64;
2626

27-
import jdk.graal.compiler.api.replacements.Fold;
2827
import org.graalvm.nativeimage.Platform;
2928
import org.graalvm.nativeimage.Platforms;
3029
import org.graalvm.nativeimage.c.function.CodePointer;
@@ -34,6 +33,8 @@
3433
import com.oracle.svm.core.Uninterruptible;
3534
import com.oracle.svm.core.feature.AutomaticallyRegisteredImageSingleton;
3635

36+
import jdk.graal.compiler.api.replacements.Fold;
37+
3738
@AutomaticallyRegisteredImageSingleton(FrameAccess.class)
3839
@Platforms(Platform.RISCV64.class)
3940
public class RISCV64FrameAccess extends FrameAccess {
@@ -56,13 +57,6 @@ public Pointer getReturnAddressLocation(Pointer sourceSp) {
5657
return sourceSp.subtract(returnAddressSize());
5758
}
5859

59-
@Fold
60-
@Override
61-
public int savedBasePointerSize() {
62-
// The base pointer is always pushed on the stack on method entry
63-
return wordSize();
64-
}
65-
6660
@Override
6761
@Fold
6862
public int stackPointerAdjustmentOnCall() {

0 commit comments

Comments
 (0)