From 66693ea6e253bddd07a288bef4bcdc015cf13b1f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Crelier?= <regis@google.com>
Date: Tue, 7 Aug 2018 19:06:48 +0000
Subject: [PATCH] [VM runtime] For now, do not use field guards when using
 kernel bytecode.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Change-Id: I6619d329eda70a4fd684f2c2bed4ba2934d1271f
Reviewed-on: https://dart-review.googlesource.com/68540
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Régis Crelier <regis@google.com>
---
 runtime/vm/flag_list.h    |  9 ++++++-
 runtime/vm/interpreter.cc | 55 +++++++++++++++++++++++----------------
 2 files changed, 41 insertions(+), 23 deletions(-)

diff --git a/runtime/vm/flag_list.h b/runtime/vm/flag_list.h
index 40a8d2896bc..1e74d39ae06 100644
--- a/runtime/vm/flag_list.h
+++ b/runtime/vm/flag_list.h
@@ -12,6 +12,13 @@
 #define USING_DBC false
 #endif
 
+// Don't use USING_KBC outside of this file.
+#if defined(DART_USE_INTERPRETER)
+#define USING_KBC true
+#else
+#define USING_KBC false
+#endif
+
 // Don't use USING_MULTICORE outside of this file.
 #if defined(ARCH_IS_MULTI_CORE)
 #define USING_MULTICORE true
@@ -186,7 +193,7 @@ constexpr bool kDartPrecompiledRuntime = false;
   P(use_compactor, bool, false, "Compact the heap during old-space GC.")       \
   P(use_cha_deopt, bool, true,                                                 \
     "Use class hierarchy analysis even if it can cause deoptimization.")       \
-  P(use_field_guards, bool, !USING_DBC,                                        \
+  P(use_field_guards, bool, !USING_DBC && !USING_KBC,                          \
     "Use field guards and track field types")                                  \
   C(use_osr, false, true, bool, true, "Use OSR")                               \
   P(use_strong_mode_types, bool, true, "Optimize based on strong mode types.") \
diff --git a/runtime/vm/interpreter.cc b/runtime/vm/interpreter.cc
index 0f2cea16f26..34c4b8c5ead 100644
--- a/runtime/vm/interpreter.cc
+++ b/runtime/vm/interpreter.cc
@@ -1013,28 +1013,30 @@ DART_NOINLINE bool Interpreter::ProcessInvocation(bool* invoked,
           return false;
         }
       }
-      // Check value cid according to field.guarded_cid().
-      // The interpreter should never see a cloned field.
-      ASSERT(field->ptr()->owner_->GetClassId() != kFieldCid);
-      const classid_t field_guarded_cid = field->ptr()->guarded_cid_;
-      const classid_t field_nullability_cid = field->ptr()->is_nullable_;
-      const classid_t value_cid = InterpreterHelpers::GetClassId(value);
-      if (value_cid != field_guarded_cid &&
-          value_cid != field_nullability_cid) {
-        if (Smi::Value(field->ptr()->guarded_list_length_) <
-                Field::kUnknownFixedLength &&
-            field_guarded_cid == kIllegalCid) {
-          field->ptr()->guarded_cid_ = value_cid;
-          field->ptr()->is_nullable_ = value_cid;
-        } else if (field_guarded_cid != kDynamicCid) {
-          call_top[1] = 0;  // Unused result of runtime call.
-          call_top[2] = field;
-          call_top[3] = value;
-          Exit(thread, *FP, call_top + 4, *pc);
-          NativeArguments native_args(thread, 2, call_top + 2, call_top + 1);
-          if (!InvokeRuntime(thread, this, DRT_UpdateFieldCid, native_args)) {
-            *invoked = true;
-            return false;
+      if (thread->isolate()->use_field_guards()) {
+        // Check value cid according to field.guarded_cid().
+        // The interpreter should never see a cloned field.
+        ASSERT(field->ptr()->owner_->GetClassId() != kFieldCid);
+        const classid_t field_guarded_cid = field->ptr()->guarded_cid_;
+        const classid_t field_nullability_cid = field->ptr()->is_nullable_;
+        const classid_t value_cid = InterpreterHelpers::GetClassId(value);
+        if (value_cid != field_guarded_cid &&
+            value_cid != field_nullability_cid) {
+          if (Smi::Value(field->ptr()->guarded_list_length_) <
+                  Field::kUnknownFixedLength &&
+              field_guarded_cid == kIllegalCid) {
+            field->ptr()->guarded_cid_ = value_cid;
+            field->ptr()->is_nullable_ = value_cid;
+          } else if (field_guarded_cid != kDynamicCid) {
+            call_top[1] = 0;  // Unused result of runtime call.
+            call_top[2] = field;
+            call_top[3] = value;
+            Exit(thread, *FP, call_top + 4, *pc);
+            NativeArguments native_args(thread, 2, call_top + 2, call_top + 1);
+            if (!InvokeRuntime(thread, this, DRT_UpdateFieldCid, native_args)) {
+              *invoked = true;
+              return false;
+            }
           }
         }
       }
@@ -3480,6 +3482,9 @@ RawObject* Interpreter::Call(RawFunction* function,
     RawInstance* instance = reinterpret_cast<RawInstance*>(FP[rA]);
     RawObject* value = FP[value_reg];
 
+    // TODO(regis): Implement cid guard.
+    ASSERT(!thread->isolate()->use_field_guards());
+
     instance->StorePointer(
         reinterpret_cast<RawObject**>(instance->ptr()) + offset_in_words,
         value);
@@ -3493,6 +3498,8 @@ RawObject* Interpreter::Call(RawFunction* function,
     RawInstance* instance = reinterpret_cast<RawInstance*>(FP[rA]);
     RawObject* value = FP[rD];
 
+    UNREACHABLE();  // TODO(regis): unused, remove.
+
     instance->StorePointer(
         reinterpret_cast<RawObject**>(instance->ptr()) + offset_in_words,
         value);
@@ -3506,6 +3513,10 @@ RawObject* Interpreter::Call(RawFunction* function,
     RawInstance* instance = reinterpret_cast<RawInstance*>(SP[-1]);
     RawObject* value = reinterpret_cast<RawObject*>(SP[0]);
     SP -= 2;  // Drop instance and value.
+
+    // TODO(regis): Implement cid guard.
+    ASSERT(!thread->isolate()->use_field_guards());
+
     instance->StorePointer(
         reinterpret_cast<RawObject**>(instance->ptr()) + offset_in_words,
         value);