[GR-62814] Don't use a base pointer for JIT compiled code and deopt targets.

PullRequest: graal/21490

Author: christianhaeubl

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/lir/framemap/FrameMap.java

@@ -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());

substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/FramePointerPhase.java

@@ -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;
@@ -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;
@@ -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()) {

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

@@ -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    |
@@ -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             |
@@ -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());
@@ -1704,6 +1698,7 @@ public void finish() {
          * return address.
          */
         private void allocateFramePointerSaveArea() {
+            assert framePointerSaveAreaOffset == -1;
             int framePointerSaveAreaSize = getTarget().wordSize;
             if (preserveFramePointer()) {
                 framePointerSaveAreaSize += returnAddressSize();
@@ -1724,6 +1719,7 @@ boolean needsFramePointer() {
 
         int getFramePointerSaveAreaOffset() {
             assert needsFramePointer() : "no frame pointer save area";
+            assert framePointerSaveAreaOffset != -1;
             return framePointerSaveAreaOffset;
         }
     }

substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/VerifyFramePointerPhase.java

@@ -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)) {

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/code/SharedCompilationResult.java

@@ -45,16 +45,23 @@ 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;
     }
 
     public static int getCodeAlignment(CompilationResult compilation) {

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/WindowsUnwindInfoFeature.java

@@ -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;