DBC: Eager deoptimization and CheckSmi instruction.

runtime/vm/simulator_dbc.cc

 // Copyright (c) 2016, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include <setjmp.h>  // NOLINT
 #include <stdlib.h>
 
 #include "vm/globals.h"
 #if defined(TARGET_ARCH_DBC)
 
@@ skipping 1811 matching lines @@
       Exit(thread, FP, SP + 2, pc);
       NativeArguments args(thread, 1, SP + 1, SP);
       INVOKE_RUNTIME(DRT_NonBoolTypeError, args);
     }
 
   AssertBooleanOk:
     DISPATCH();
   }
 
   {
+    BYTECODE(CheckSmi, 0);
+    intptr_t obj = reinterpret_cast<intptr_t>(FP[rA]);
+    if ((obj & kSmiTagMask) == kSmiTag) {
+      pc++;
+    }
+    DISPATCH();
+  }
+
+  {
     BYTECODE(IfEqStrictTOS, 0);
     SP -= 2;
     if (SP[1] != SP[2]) {
       pc++;
     }
     DISPATCH();
   }
 
   {
     BYTECODE(IfNeStrictTOS, 0);
@@ skipping 116 matching lines @@
     RawObject* value = FP[rC];
     ASSERT(array->GetClassId() == kArrayCid);
     ASSERT(!index->IsHeapObject());
     ASSERT(SimulatorHelpers::CheckIndex(index, array->ptr()->length_));
     array->StorePointer(array->ptr()->data() + Smi::Value(index), value);
     DISPATCH();
   }
 
   {
     BYTECODE(Deopt, A_D);
     const uint16_t deopt_id = rD;

[Vyacheslav Egorov (Google), 2016/06/09 15:09:06]

My original idea was to use this value to look up deopt information, instead of
searching by PC - but now I realize that it would be too much work.

So we can just change it's meaning to mean 'is eager' because currently it's not
used for anything. 

const bool is_lazy = rD == 0; or 

const bool is_eager = rD != 0;

depending on what you find more readable.

[zra, 2016/06/09 16:42:23]

Done.

-    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];
+    const bool preserve_result = (deopt_id == 0);

[Vyacheslav Egorov (Google), 2016/06/09 15:09:06]

I suggest removing preserve_result and use either is_eager or is_lazy.

[zra, 2016/06/09 16:42:23]

Done.

 
-      // 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);
+    // 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];
 
-      SP = FP + (frame_size_in_bytes / kWordSize);
-      EnterSyntheticFrame(&FP, &SP, pc - 1);
-      DLRT_DeoptimizeFillFrame(reinterpret_cast<uword>(FP));
+    // When not preserving the result, we still need to preserve SP[0] as it
+    // contains some temporary expression.
+    if (!preserve_result) {
+      SP++;
+    }
 
-      // We are now inside a valid frame.
-      {
+    // 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);

[Vyacheslav Egorov (Google), 2016/06/09 15:09:06]

I overlooked that this should say 

pc - is_eager ? 0 : 1.

We are going one backwards for lazy deopts because code looks like this:

.... InstanceCall
.... Deopt
PC->

and deopt information is actually recorded for PC immediately after the call.

However for normal eager deopts we should record deopt information for PC
directly after Deopt instruction so PC-1 is not needed.

[zra, 2016/06/09 16:42:23]

Done.

+    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.
+      *++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);
+    // Restore caller PC.
+    pc = SavedCallerPC(FP);
 
-      // Check if it is a fake PC marking the entry frame.
-      ASSERT((reinterpret_cast<uword>(pc) & 2) == 0);
+    // 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();
+    // 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.
+
+    // If the result is not preserved, the unoptimized frame ends at the
+    // next slot.
+    SP = FrameArguments(FP,
+        materialization_arg_count + (preserve_result ? 0 : 1));

[Vyacheslav Egorov (Google), 2016/06/09 15:09:06]

I thought about it and it might be more readable to write it as 

// SP points to the result slot that used to contain the first argument.
SP = FrameArguments(FP, materialization_arg_count);


...

if (is_eager) {
  SP--;  // No result to push.
} else {
  SP[0] = result;  // Put result to the stack.
}
 

[zra, 2016/06/09 16:42:23]

Done.

+    FP = SavedCallerFP(FP);
+    pp = SimulatorHelpers::FrameCode(FP)->ptr()->object_pool_->ptr();
+    if (preserve_result) {
       *SP = result;
-    } else {
-      UNIMPLEMENTED();
     }
     DISPATCH();
   }
 
   {
     BYTECODE(Trap, 0);
     UNIMPLEMENTED();
     DISPATCH();
   }
 
@@ skipping 95 matching lines @@
   special_[kExceptionSpecialIndex] = raw_exception;
   special_[kStacktraceSpecialIndex] = raw_stacktrace;
   buf->Longjmp();
   UNREACHABLE();
 }
 
 }  // namespace dart
 
 
 #endif  // defined TARGET_ARCH_DBC