Skip to content

[GR-66193] Backport to 25.0: Don't use a base pointer for JIT compiled code and deopt targets. #11744

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.

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 1 commit into from
Jul 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ protected int alignFrameSize(int size) {
* be requested.
*/
public void finish() {
GraalError.guarantee(frameSize == -1, "frame size may only be computed once");
frameSize = currentFrameSize();
if (frameSize > getRegisterConfig().getMaximumFrameSize()) {
throw new PermanentBailoutException("Frame size (%d) exceeded maximum allowed frame size (%d).", frameSize, getRegisterConfig().getMaximumFrameSize());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.ArrayList;

import com.oracle.svm.core.SubstrateOptions;
import com.oracle.svm.core.SubstrateUtil;
import com.oracle.svm.core.graal.code.SubstrateBackend.SubstrateMarkId;

import jdk.graal.compiler.asm.amd64.AMD64MacroAssembler;
Expand All @@ -51,9 +52,13 @@
*
* @see LIRInstruction#modifiesStackPointer
*/
class FramePointerPhase extends PreAllocationOptimizationPhase {
public class FramePointerPhase extends PreAllocationOptimizationPhase {
@Override
protected void run(TargetDescription target, LIRGenerationResult lirGenRes, PreAllocationOptimizationContext context) {
if (!isSupported(lirGenRes)) {
return;
}

LIR lir = lirGenRes.getLIR();
if (!modifiesStackPointer(lir)) {
return;
Expand Down Expand Up @@ -101,6 +106,16 @@ protected void run(TargetDescription target, LIRGenerationResult lirGenRes, PreA
}
}

static boolean isSupported(LIRGenerationResult lirGenRes) {
/*
* JIT compilation and deopt targets are not supported, see GR-64771. For these unsupported
* methods, a base pointer is not used, even if there are LIR instructions that modify the
* stack pointer directly.
*/
SubstrateAMD64Backend.SubstrateLIRGenerationResult result = (SubstrateAMD64Backend.SubstrateLIRGenerationResult) lirGenRes;
return SubstrateUtil.HOSTED && !result.getMethod().isDeoptTarget();
}

/** Returns true if any LIR instruction modifies the stack pointer, false otherwise. */
private static boolean modifiesStackPointer(LIR lir) {
for (int blockId : lir.getBlocks()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1151,12 +1151,9 @@ private static ForeignCallDescriptor chooseCPUFeatureVariant(ForeignCallDescript
* If a method doesn't need a frame pointer, we use the following forms:
*
* <pre>
* | needsFramePointer |
* | needsFramePointer = false |
* +---------------------------------+
* | false |
* +---------------------------------+
* | preserveFramePointer |
* +----------------+----------------+
* | preserveFramePointer = ... |
* | false | true |
* --------+----------------+----------------+
* | ; prologue | ; prologue |
Expand Down Expand Up @@ -1187,12 +1184,9 @@ private static ForeignCallDescriptor chooseCPUFeatureVariant(ForeignCallDescript
* If a method does need a frame pointer, we use the following forms:
*
* <pre>
* | needsFramePointer |
* +---------------------------------------------------+
* | true |
* | needsFramePointer = true |
* +---------------------------------------------------+
* | preserveFramePointer |
* +-------------------------+-------------------------+
* | preserveFramePointer = ... |
* | false | true |
* --------+-------------------------+-------------------------+
* | ; prologue | ; prologue |
Expand Down Expand Up @@ -1675,7 +1669,7 @@ static class SubstrateAMD64FrameMap extends AMD64FrameMap {
private boolean needsFramePointer;

/** The offset at which the frame pointer save area is located. */
private int framePointerSaveAreaOffset;
private int framePointerSaveAreaOffset = -1;

SubstrateAMD64FrameMap(CodeCacheProvider codeCache, SubstrateAMD64RegisterConfig registerConfig, ReferenceMapBuilderFactory referenceMapFactory, SharedMethod method) {
super(codeCache, registerConfig, referenceMapFactory, registerConfig.shouldUseBasePointer());
Expand Down Expand Up @@ -1704,6 +1698,7 @@ public void finish() {
* return address.
*/
private void allocateFramePointerSaveArea() {
assert framePointerSaveAreaOffset == -1;
int framePointerSaveAreaSize = getTarget().wordSize;
if (preserveFramePointer()) {
framePointerSaveAreaSize += returnAddressSize();
Expand All @@ -1724,6 +1719,7 @@ boolean needsFramePointer() {

int getFramePointerSaveAreaOffset() {
assert needsFramePointer() : "no frame pointer save area";
assert framePointerSaveAreaOffset != -1;
return framePointerSaveAreaOffset;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@
class VerifyFramePointerPhase extends FinalCodeAnalysisPhase {
@Override
protected void run(TargetDescription target, LIRGenerationResult lirGenRes, FinalCodeAnalysisContext context) {
if (!FramePointerPhase.isSupported(lirGenRes)) {
return;
}

LIR lir = lirGenRes.getLIR();
for (int blockId : lir.getBlocks()) {
if (LIR.isBlockDeleted(blockId)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,22 @@ public int getFrameSize() {
return frameSize;
}

public void setFrameSize(int frameSize) {
this.frameSize = frameSize;
public void setFrameSize(int value) {
assert frameSize == -1;
this.frameSize = value;
}

public boolean hasFramePointerSaveAreaOffset() {
return framePointerSaveAreaOffset != -1;
}

public int getFramePointerSaveAreaOffset() {
assert hasFramePointerSaveAreaOffset();
return framePointerSaveAreaOffset;
}

public void setFramePointerSaveAreaOffset(int framePointerSaveAreaOffset) {
this.framePointerSaveAreaOffset = framePointerSaveAreaOffset;
public void setFramePointerSaveAreaOffset(int value) {
assert !hasFramePointerSaveAreaOffset();
this.framePointerSaveAreaOffset = value;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,8 @@ private static void visitRanges(CompilationResult compilation, RangeVisitor visi
return; /* No frame, no unwind info needed. */
}

int framePointerSaveAreaOffset = ((SharedCompilationResult) compilation).getFramePointerSaveAreaOffset();
if (framePointerSaveAreaOffset < 0) {
SharedCompilationResult cr = (SharedCompilationResult) compilation;
if (!cr.hasFramePointerSaveAreaOffset()) {
/* There is no frame pointer, so there is only the primary range. */
visitor.visit(RUNTIME_FUNCTION.PRIMARY_RANGE, START_MARK, compilation.getTargetCodeSize());
return;
Expand Down