[GR-68134] In deoptimization stub, save registers again and restore return address.

PullRequest: graal/21677

Author: peter-hofer

substratevm/src/com.oracle.svm.core.graal.aarch64/src/com/oracle/svm/core/graal/aarch64/AArch64FarReturnOp.java

@@ -73,6 +73,7 @@ public void emitCode(CompilationResultBuilder crb, AArch64MacroAssembler masm) {
             /* No need to restore anything in the frame of the new stack pointer. */
             masm.mov(64, AArch64.sp, asRegister(sp));
             returnTo(asRegister(ip), masm);
+            return;
         }
 
         /*

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

@@ -487,27 +487,19 @@ public SubstrateLIRGenerationResult(CompilationIdentifier compilationId, LIR lir
             super(compilationId, lir, frameMapBuilder, registerAllocationConfig, callingConvention);
             this.method = method;
 
-            /*
-             * Besides for methods with callee saved registers, we reserve additional stack space
-             * for lazyDeoptStub too. This is necessary because the lazy deopt stub might read
-             * callee-saved register values in the callee of the function to be deoptimized, thus
-             * that stack space must not be overwritten by the lazy deopt stub.
-             */
-            if (method.hasCalleeSavedRegisters() || method.getDeoptStubType() == Deoptimizer.StubType.LazyEntryStub) {
+            if (method.hasCalleeSavedRegisters()) {
                 AArch64CalleeSavedRegisters calleeSavedRegisters = AArch64CalleeSavedRegisters.singleton();
                 FrameMap frameMap = ((FrameMapBuilderTool) frameMapBuilder).getFrameMap();
                 int registerSaveAreaSizeInBytes = calleeSavedRegisters.getSaveAreaSize();
                 StackSlot calleeSaveArea = frameMap.allocateStackMemory(registerSaveAreaSizeInBytes, frameMap.getTarget().wordSize);
 
-                if (method.hasCalleeSavedRegisters()) {
-                    /*
-                     * The offset of the callee save area must be fixed early during image
-                     * generation. It is accessed when compiling methods that have a call with
-                     * callee-saved calling convention. Here we verify that offset computed earlier
-                     * is the same as the offset actually reserved.
-                     */
-                    calleeSavedRegisters.verifySaveAreaOffsetInFrame(calleeSaveArea.getRawOffset());
-                }
+                /*
+                 * The offset of the callee save area must be fixed early during image generation.
+                 * It is accessed when compiling methods that have a call with callee-saved calling
+                 * convention. Here we verify that offset computed earlier is the same as the offset
+                 * actually reserved.
+                 */
+                calleeSavedRegisters.verifySaveAreaOffsetInFrame(calleeSaveArea.getRawOffset());
             }
 
             if (method.canDeoptimize() || method.isDeoptTarget()) {
@@ -1042,9 +1034,8 @@ public void returned(CompilationResultBuilder crb) {
     }
 
     /**
-     * Generates the prologue of a
-     * {@link com.oracle.svm.core.deopt.Deoptimizer.StubType#EagerEntryStub} or
-     * {@link com.oracle.svm.core.deopt.Deoptimizer.StubType#LazyEntryStub} method.
+     * Generates the prologue of a {@link com.oracle.svm.core.deopt.Deoptimizer.StubType#EntryStub}
+     * method.
      */
     protected static class DeoptEntryStubContext extends SubstrateAArch64FrameContext {
         protected final CallingConvention callingConvention;
@@ -1058,9 +1049,18 @@ protected DeoptEntryStubContext(SharedMethod method, CallingConvention callingCo
         public void enter(CompilationResultBuilder crb) {
             AArch64MacroAssembler masm = (AArch64MacroAssembler) crb.asm;
             RegisterConfig registerConfig = crb.frameMap.getRegisterConfig();
+            Register frameRegister = registerConfig.getFrameRegister();
             Register gpReturnReg = registerConfig.getReturnRegister(JavaKind.Object);
             Register fpReturnReg = registerConfig.getReturnRegister(JavaKind.Double);
 
+            /* Create the frame. */
+            super.enter(crb);
+
+            /*
+             * Synthesize the parameters for the deopt stub. This needs to be done after enter() to
+             * avoid overwriting register values that it might save to the stack.
+             */
+
             /* Pass the general purpose and floating point registers to the deopt stub. */
             Register secondParameter = ValueUtil.asRegister(callingConvention.getArgument(1));
             masm.mov(64, secondParameter, gpReturnReg);
@@ -1073,9 +1073,7 @@ public void enter(CompilationResultBuilder crb) {
              * the first argument register may overlap with the object return register.
              */
             Register firstParameter = ValueUtil.asRegister(callingConvention.getArgument(0));
-            masm.mov(64, firstParameter, registerConfig.getFrameRegister());
-
-            super.enter(crb);
+            masm.add(64, firstParameter, frameRegister, crb.frameMap.totalFrameSize());
         }
     }
 
@@ -1343,7 +1341,7 @@ public CompilationResultBuilder newCompilationResultBuilder(LIRGenerationResult
     }
 
     protected FrameContext createFrameContext(SharedMethod method, Deoptimizer.StubType stubType, CallingConvention callingConvention) {
-        if (stubType == Deoptimizer.StubType.EagerEntryStub || stubType == Deoptimizer.StubType.LazyEntryStub) {
+        if (stubType == Deoptimizer.StubType.EntryStub) {
             return new DeoptEntryStubContext(method, callingConvention);
         } else if (stubType == Deoptimizer.StubType.ExitStub) {
             return new DeoptExitStubContext(method, callingConvention);

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

@@ -564,27 +564,19 @@ public SubstrateLIRGenerationResult(CompilationIdentifier compilationId, LIR lir
             super(compilationId, lir, frameMapBuilder, registerAllocationConfig, callingConvention);
             this.method = method;
 
-            /*
-             * Besides for methods with callee saved registers, we reserve additional stack space
-             * for lazyDeoptStub too. This is necessary because the lazy deopt stub might read
-             * callee-saved register values in the callee of the function to be deoptimized, thus
-             * that stack space must not be overwritten by the lazy deopt stub.
-             */
-            if (method.hasCalleeSavedRegisters() || method.getDeoptStubType() == Deoptimizer.StubType.LazyEntryStub) {
+            if (method.hasCalleeSavedRegisters()) {
                 AMD64CalleeSavedRegisters calleeSavedRegisters = AMD64CalleeSavedRegisters.singleton();
                 FrameMap frameMap = ((FrameMapBuilderTool) frameMapBuilder).getFrameMap();
                 int registerSaveAreaSizeInBytes = calleeSavedRegisters.getSaveAreaSize();
                 StackSlot calleeSaveArea = frameMap.allocateStackMemory(registerSaveAreaSizeInBytes, frameMap.getTarget().wordSize);
 
-                if (method.hasCalleeSavedRegisters()) {
-                    /*
-                     * The offset of the callee save area must be fixed early during image
-                     * generation. It is accessed when compiling methods that have a call with
-                     * callee-saved calling convention. Here we verify that offset computed earlier
-                     * is the same as the offset actually reserved.
-                     */
-                    calleeSavedRegisters.verifySaveAreaOffsetInFrame(calleeSaveArea.getRawOffset());
-                }
+                /*
+                 * The offset of the callee save area must be fixed early during image generation.
+                 * It is accessed when compiling methods that have a call with callee-saved calling
+                 * convention. Here we verify that offset computed earlier is the same as the offset
+                 * actually reserved.
+                 */
+                calleeSavedRegisters.verifySaveAreaOffsetInFrame(calleeSaveArea.getRawOffset());
             }
 
             if (method.canDeoptimize() || method.isDeoptTarget()) {
@@ -1344,9 +1336,8 @@ public void returned(CompilationResultBuilder crb) {
     }
 
     /**
-     * Generates the prologue of a
-     * {@link com.oracle.svm.core.deopt.Deoptimizer.StubType#EagerEntryStub} or
-     * {@link com.oracle.svm.core.deopt.Deoptimizer.StubType#LazyEntryStub} method.
+     * Generates the prologue of a {@link com.oracle.svm.core.deopt.Deoptimizer.StubType#EntryStub}
+     * method.
      */
     protected static class DeoptEntryStubContext extends SubstrateAMD64FrameContext {
         protected DeoptEntryStubContext(SharedMethod method, CallingConvention callingConvention) {
@@ -1357,33 +1348,39 @@ protected DeoptEntryStubContext(SharedMethod method, CallingConvention callingCo
         public void enter(CompilationResultBuilder tasm) {
             AMD64MacroAssembler asm = (AMD64MacroAssembler) tasm.asm;
             RegisterConfig registerConfig = tasm.frameMap.getRegisterConfig();
+            Register frameRegister = registerConfig.getFrameRegister();
             Register gpReturnReg = registerConfig.getReturnRegister(JavaKind.Long);
             Register fpReturnReg = registerConfig.getReturnRegister(JavaKind.Double);
-
             Register firstArgument = ValueUtil.asRegister(callingConvention.getArgument(0));
             assert !firstArgument.equals(gpReturnReg) : "overwriting return register";
+
             /*
              * Since this is the target for all deoptimizations we must mark the start of this
              * routine as an indirect target.
              */
             asm.maybeEmitIndirectTargetMarker();
 
-            /* Pass the address of the frame to deoptimize as first argument. */
-            asm.movq(firstArgument, registerConfig.getFrameRegister());
-
-            /* Copy the original return registers values into the argument registers. */
-            asm.movq(ValueUtil.asRegister(callingConvention.getArgument(1)), gpReturnReg);
-            asm.movdq(ValueUtil.asRegister(callingConvention.getArgument(2)), fpReturnReg);
-
             /*
-             * Keep the return address slot. This keeps the stack walkable, which is crucial for the
-             * interruptible phase of lazy deoptimization. (The return address points to the deopt
-             * stub, while the original return address is stored in the deopt slot.)
+             * Keep the return address slot. The correct return address is written in the stub
+             * itself (read more there). The original return address is stored in the deopt slot.
              *
-             * This also ensures that the stack pointer is aligned properly.
+             * Keeping this slot also ensures that the stack pointer is aligned properly.
              */
             asm.subq(registerConfig.getFrameRegister(), FrameAccess.returnAddressSize());
+
             super.enter(tasm);
+
+            /*
+             * Synthesize the parameters for the deopt stub. This needs to be done after enter() to
+             * avoid overwriting register values that it might save to the stack.
+             */
+
+            /* Pass the address of the frame to deoptimize as first argument. */
+            asm.leaq(firstArgument, new AMD64Address(frameRegister, tasm.frameMap.totalFrameSize()));
+
+            /* Copy the original return registers values into the argument registers. */
+            asm.movq(ValueUtil.asRegister(callingConvention.getArgument(1)), gpReturnReg);
+            asm.movdq(ValueUtil.asRegister(callingConvention.getArgument(2)), fpReturnReg);
         }
     }
 
@@ -1392,7 +1389,7 @@ public void enter(CompilationResultBuilder tasm) {
      * method.
      *
      * Note no special handling is necessary for CFI as this will be a direct call from the
-     * {@link com.oracle.svm.core.deopt.Deoptimizer.StubType#EagerEntryStub}.
+     * {@link com.oracle.svm.core.deopt.Deoptimizer.StubType#EntryStub}.
      */
     protected static class DeoptExitStubContext extends SubstrateAMD64FrameContext {
         protected DeoptExitStubContext(SharedMethod method, CallingConvention callingConvention) {
@@ -1918,7 +1915,7 @@ protected AMD64MacroAssembler createAssembler(OptionValues options) {
     }
 
     protected FrameContext createFrameContext(SharedMethod method, Deoptimizer.StubType stubType, CallingConvention callingConvention) {
-        if (stubType == Deoptimizer.StubType.EagerEntryStub || stubType == Deoptimizer.StubType.LazyEntryStub) {
+        if (stubType == Deoptimizer.StubType.EntryStub) {
             return new DeoptEntryStubContext(method, callingConvention);
         } else if (stubType == Deoptimizer.StubType.ExitStub) {
             return new DeoptExitStubContext(method, callingConvention);