DBC: Eager deoptimization and CheckSmi instruction.

runtime/vm/simulator_dbc.cc

Index: runtime/vm/simulator_dbc.cc
diff --git a/runtime/vm/simulator_dbc.cc b/runtime/vm/simulator_dbc.cc
index baafca844ed13c442ee0d7286b0eec59e7583fd2..1534fd7b4a83d4be297d6a443b6fc9779fea71a7 100644
--- a/runtime/vm/simulator_dbc.cc
+++ b/runtime/vm/simulator_dbc.cc
@@ -1966,54 +1966,59 @@ RawObject* Simulator::Call(const Code& code,
   {
     BYTECODE(Deopt, A_D);
     const uint16_t deopt_id = rD;
-    if (deopt_id == 0) {  // Lazy deoptimization.
-      // Preserve result of the previous call.
-      // TODO(vegorov) we could have actually included result into the
-      // deoptimization environment because it is passed through the stack.
-      // If we do then we could remove special result handling from this code.
-      RawObject* result = SP[0];
-
-      // Leaf runtime function DeoptimizeCopyFrame expects a Dart frame.
-      // The code in this frame may not cause GC.
-      // DeoptimizeCopyFrame and DeoptimizeFillFrame are leaf runtime calls.
-      EnterSyntheticFrame(&FP, &SP, pc - 1);
-      const intptr_t frame_size_in_bytes =
-          DLRT_DeoptimizeCopyFrame(reinterpret_cast<uword>(FP),
-                                   /*is_lazy_deopt=*/1);
-      LeaveSyntheticFrame(&FP, &SP);
-
-      SP = FP + (frame_size_in_bytes / kWordSize);
-      EnterSyntheticFrame(&FP, &SP, pc - 1);
-      DLRT_DeoptimizeFillFrame(reinterpret_cast<uword>(FP));
-
-      // We are now inside a valid frame.
-      {
+    const bool preserve_result = (deopt_id == 0);
+
+    // Preserve result of the previous call.
+    // TODO(vegorov) we could have actually included result into the
+    // deoptimization environment because it is passed through the stack.
+    // If we do then we could remove special result handling from this code.
+    RawObject* result = SP[0];

[Vyacheslav Egorov (Google), 2016/06/08 11:58:53]

When not preserving result (non-lazy deopt) we need to preserve SP[0] - it does
not contain result, but contains some temporary expression. So you need to do

if (!preserve_result) SP++; 

before entering the synthetic frame, otherwise SP[0] will be damaged.

[zra, 2016/06/08 17:46:30]

This and the fix below made things work.

+
+    // Leaf runtime function DeoptimizeCopyFrame expects a Dart frame.
+    // The code in this frame may not cause GC.
+    // DeoptimizeCopyFrame and DeoptimizeFillFrame are leaf runtime calls.
+    EnterSyntheticFrame(&FP, &SP, pc - 1);

[zra, 2016/06/07 22:31:29]

I'm not sure I see how the locals from the optimized frame get shuffled into the
locals for the unoptimized frame...

[Vyacheslav Egorov (Google), 2016/06/08 11:58:53]

If you look at the disassembled optimized code object then you will see that
each deoptimization point comes with a deoptimization environment attached. It
looks like this:

   0: 0x7f64ed3ee6c4  [callerfp][retaddr(1,
6)][pc(2)][const(2)][word(R0)][const(3)][const(4)][const(3)][callerfp][callerpc][pc(1)][const(1)][const(3)]


This is encoding of the stack state in terms of constants and values from the
optimized frame, e.g. [word(R0)] says "take R0 from optimized frame and put it
here".

Deoptimizer removes optimized frame from the stack and them reconstructs
unoptimized frame(s) using this description. 

[zra, 2016/06/08 17:46:30]

Thanks!

+    const intptr_t frame_size_in_bytes =
+        DLRT_DeoptimizeCopyFrame(reinterpret_cast<uword>(FP),
+                                 /*is_lazy_deopt=*/ (deopt_id == 0) ? 1 : 0);
+    LeaveSyntheticFrame(&FP, &SP);
+
+    SP = FP + (frame_size_in_bytes / kWordSize);
+    EnterSyntheticFrame(&FP, &SP, pc - 1);
+    DLRT_DeoptimizeFillFrame(reinterpret_cast<uword>(FP));
+
+    // We are now inside a valid frame.
+    {
+      if (preserve_result) {
         *++SP = result;  // Preserve result (call below can cause GC).
-        *++SP = 0;  // Space for the result: number of materialization args.
-        Exit(thread, FP, SP + 1, /*pc=*/0);
-        NativeArguments native_args(thread, 0, SP, SP);
-        INVOKE_RUNTIME(DRT_DeoptimizeMaterialize, native_args);
       }
-      const intptr_t materialization_arg_count =
-          Smi::Value(RAW_CAST(Smi, *SP--));
-      result = *SP--;  // Reload the result. It might have been relocated by GC.
-
-      // Restore caller PC.
-      pc = SavedCallerPC(FP);
-
-      // Check if it is a fake PC marking the entry frame.
-      ASSERT((reinterpret_cast<uword>(pc) & 2) == 0);
-
-      // Restore SP, FP and PP. Push result and dispatch.
-      // Note: unlike in a normal return sequence we don't need to drop
-      // arguments - those are not part of the innermost deoptimization
-      // environment they were dropped by FlowGraphCompiler::RecordAfterCall.
-      SP = FrameArguments(FP, materialization_arg_count);
-      FP = SavedCallerFP(FP);
-      pp = SimulatorHelpers::FrameCode(FP)->ptr()->object_pool_->ptr();
+      *++SP = 0;  // Space for the result: number of materialization args.
+      Exit(thread, FP, SP + 1, /*pc=*/0);
+      NativeArguments native_args(thread, 0, SP, SP);
+      INVOKE_RUNTIME(DRT_DeoptimizeMaterialize, native_args);
+    }
+    const intptr_t materialization_arg_count =
+        Smi::Value(RAW_CAST(Smi, *SP--));
+    if (preserve_result) {
+      // Reload the result. It might have been relocated by GC.
+      result = *SP--;
+    }
+
+    // Restore caller PC.
+    pc = SavedCallerPC(FP);
+
+    // Check if it is a fake PC marking the entry frame.
+    ASSERT((reinterpret_cast<uword>(pc) & 2) == 0);
+
+    // Restore SP, FP and PP. Push result and dispatch.
+    // Note: unlike in a normal return sequence we don't need to drop
+    // arguments - those are not part of the innermost deoptimization
+    // environment they were dropped by FlowGraphCompiler::RecordAfterCall.
+    SP = FrameArguments(FP, materialization_arg_count);

[Vyacheslav Egorov (Google), 2016/06/08 11:58:53]

FrameArguments(FP, argc) points to the first argument. Lets look at the stack
layout right after deoptimizer finishes writing it to the stack (can be seen
with --trace-deoptimization-verbose)

*0. [0x2fcf3c8] 0x2fcf390 [callerfp]      \ <- FP
*1. [0x2fcf3c0] 0x0 [retaddr(1, 6)]       |
*2. [0x2fcf3b8] 0x7f64ef3eb171 [pc(2)]    |
*3. [0x2fcf3b0] 0x7f64ef3e8021 [const(2)] /
*4. [0x2fcf3a8] 0x7f64ee014981 [word(R0)] \
*5. [0x2fcf3a0] 0x7f64ee053781 [const(3)] |
*6. [0x2fcf398] 0x7f64ee0ec051 [const(4)] |
*7. [0x2fcf390] 0x7f64ee053781 [const(3)] |
*8. [0x2fcf388] 0x2fcf348 [callerfp]      |
*9. [0x2fcf380] 0x7f64ed3ee3d0 [callerpc] |
*10. [0x2fcf378] 0x7f64ed3c71e1 [pc(1)]   |
*11. [0x2fcf370] 0x7f64ed6b35c1 [const(1)]|
*12. [0x2fcf368] 0x7f64ee053781 [const(3)]/

Slots 0 - 3 are synthetic frame (needed for GC functioning) slots 4 to 12 are
our newly reconstructed unoptimized frame. 

FP is pointing to the slot 0. materialization_arg_count is 0.

If you do SP = FrameArguments(FP, 0) you get SP pointing to the slot 3. 

This would be fine if we were doing a lazy deopt - because we are going to set
SP[0] to result of the function call and continue. 

However for eager deopt we are not going to push any result. 

This means we should set SP to slot 4, not slot 3. Because slot 4 is precisely
where stack frame for unoptimized code ends.

So code should say

SP = FrameArguments(FP, materialization_arg_count + (preserve_result ? 0 : 1))

to restore the correct SP.

[zra, 2016/06/08 17:46:30]

That makes sense, thanks.

+    FP = SavedCallerFP(FP);
+    pp = SimulatorHelpers::FrameCode(FP)->ptr()->object_pool_->ptr();
+    if (preserve_result) {
       *SP = result;
-    } else {
-      UNIMPLEMENTED();
     }
     DISPATCH();
   }