Revert "[vm] Enable type stubs based type checks in JIT mode for some types."

This reverts commit 4be50d6.

Reason for revert: Failures on SIMDBC64 and Analyzer bots.

Original change's description:
> [vm] Enable type stubs based type checks in JIT mode for some types.
> 
> For now we are limiting this to type checks against type parameter types.
> 
> # Performance improvements
> 
> In Dart 1 mode Dart2JS compiles itself in 28s when running from source
> and in 23s when running from ideal app-jit snapshot (trained on the
> same workload).
> 
> Before this change in Dart 2 mode numbers were 51s and 57s respectively.
> 
> After this change in Dart 2 mode numbers are 38s and 32s. Meaning
> that regression is reduced by 50%.
> 
> Issue #31798
> Issue #33257
> 
> Change-Id: I34bf5385a5cc3c7702dc281c6dfa89da85d3dde1
> Reviewed-on: https://dart-review.googlesource.com/57601
> Reviewed-by: Régis Crelier <[email protected]>
> Commit-Queue: Vyacheslav Egorov <[email protected]>

[email protected],[email protected],[email protected]

Change-Id: I85a30c962b0cd556310e19193f5993ab76ecf2e7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/57840
Reviewed-by: Vyacheslav Egorov <[email protected]>
Commit-Queue: Vyacheslav Egorov <[email protected]>

Author: mraleph

runtime/vm/clustered_snapshot.cc

@@ -33,10 +33,6 @@ static RawObject* AllocateUninitialized(PageSpace* old_space, intptr_t size) {
   return RawObject::FromAddr(address);
 }
 
-static bool SnapshotContainsTypeTestingStubs(Snapshot::Kind kind) {
-  return kind == Snapshot::kFullAOT || kind == Snapshot::kFullJIT;
-}
-
 void Deserializer::InitializeHeader(RawObject* raw,
                                     intptr_t class_id,
                                     intptr_t size,
@@ -3062,8 +3058,6 @@ class TypeSerializationCluster : public SerializationCluster {
 
   void WriteFill(Serializer* s) {
     const bool is_vm_isolate = s->isolate() == Dart::vm_isolate();
-    const bool should_write_type_testing_stub =
-        SnapshotContainsTypeTestingStubs(s->kind());
 
     intptr_t count = canonical_objects_.length();
     for (intptr_t i = 0; i < count; i++) {
@@ -3075,7 +3069,7 @@ class TypeSerializationCluster : public SerializationCluster {
       }
       s->WriteTokenPosition(type->ptr()->token_pos_);
       s->Write<int8_t>(type->ptr()->type_state_);
-      if (should_write_type_testing_stub) {
+      if (s->kind() == Snapshot::kFullAOT) {
         RawInstructions* instr = type_testing_stubs_.LookupByAddresss(
             type->ptr()->type_test_stub_entry_point_);
         s->WriteInstructions(instr, Code::null());
@@ -3091,7 +3085,7 @@ class TypeSerializationCluster : public SerializationCluster {
       }
       s->WriteTokenPosition(type->ptr()->token_pos_);
       s->Write<int8_t>(type->ptr()->type_state_);
-      if (should_write_type_testing_stub) {
+      if (s->kind() == Snapshot::kFullAOT) {
         RawInstructions* instr = type_testing_stubs_.LookupByAddresss(
             type->ptr()->type_test_stub_entry_point_);
         s->WriteInstructions(instr, Code::null());
@@ -3100,7 +3094,7 @@ class TypeSerializationCluster : public SerializationCluster {
 
     // The dynamic/void objects are not serialized, so we manually send
     // the type testing stub for it.
-    if (should_write_type_testing_stub && is_vm_isolate) {
+    if (s->kind() == Snapshot::kFullAOT && is_vm_isolate) {
       RawInstructions* dynamic_instr = type_testing_stubs_.LookupByAddresss(
           Type::dynamic_type().type_test_stub_entry_point());
       s->WriteInstructions(dynamic_instr, Code::null());
@@ -3143,8 +3137,6 @@ class TypeDeserializationCluster : public DeserializationCluster {
 
   void ReadFill(Deserializer* d) {
     const bool is_vm_isolate = d->isolate() == Dart::vm_isolate();
-    const bool should_read_type_testing_stub =
-        SnapshotContainsTypeTestingStubs(d->kind());
 
     for (intptr_t id = canonical_start_index_; id < canonical_stop_index_;
          id++) {
@@ -3158,7 +3150,7 @@ class TypeDeserializationCluster : public DeserializationCluster {
       }
       type->ptr()->token_pos_ = d->ReadTokenPosition();
       type->ptr()->type_state_ = d->Read<int8_t>();
-      if (should_read_type_testing_stub) {
+      if (d->kind() == Snapshot::kFullAOT) {
         instr_ = d->ReadInstructions();
         type_ = type;
         type_.SetTypeTestingStub(instr_);
@@ -3176,7 +3168,7 @@ class TypeDeserializationCluster : public DeserializationCluster {
       }
       type->ptr()->token_pos_ = d->ReadTokenPosition();
       type->ptr()->type_state_ = d->Read<int8_t>();
-      if (should_read_type_testing_stub) {
+      if (d->kind() == Snapshot::kFullAOT) {
         instr_ = d->ReadInstructions();
         type_ = type;
         type_.SetTypeTestingStub(instr_);
@@ -3185,7 +3177,7 @@ class TypeDeserializationCluster : public DeserializationCluster {
 
     // The dynamic/void objects are not serialized, so we manually send
     // the type testing stub for it.
-    if (should_read_type_testing_stub && is_vm_isolate) {
+    if (d->kind() == Snapshot::kFullAOT && is_vm_isolate) {
       instr_ = d->ReadInstructions();
       Type::dynamic_type().SetTypeTestingStub(instr_);
       instr_ = d->ReadInstructions();
@@ -3194,7 +3186,7 @@ class TypeDeserializationCluster : public DeserializationCluster {
   }
 
   void PostLoad(const Array& refs, Snapshot::Kind kind, Zone* zone) {
-    if (!SnapshotContainsTypeTestingStubs(kind)) {
+    if (kind != Snapshot::kFullAOT) {
       for (intptr_t id = canonical_start_index_; id < canonical_stop_index_;
            id++) {
         type_ ^= refs.At(id);
@@ -3245,9 +3237,6 @@ class TypeRefSerializationCluster : public SerializationCluster {
   }
 
   void WriteFill(Serializer* s) {
-    const bool should_write_type_testing_stub =
-        SnapshotContainsTypeTestingStubs(s->kind());
-
     intptr_t count = objects_.length();
     for (intptr_t i = 0; i < count; i++) {
       RawTypeRef* type = objects_[i];
@@ -3256,7 +3245,7 @@ class TypeRefSerializationCluster : public SerializationCluster {
       for (RawObject** p = from; p <= to; p++) {
         s->WriteRef(*p);
       }
-      if (should_write_type_testing_stub) {
+      if (s->kind() == Snapshot::kFullAOT) {
         RawInstructions* instr = type_testing_stubs_.LookupByAddresss(
             type->ptr()->type_test_stub_entry_point_);
         s->WriteInstructions(instr, Code::null());
@@ -3287,9 +3276,7 @@ class TypeRefDeserializationCluster : public DeserializationCluster {
   }
 
   void ReadFill(Deserializer* d) {
-    const bool is_vm_object = d->isolate() == Dart::vm_isolate();
-    const bool should_read_type_testing_stub =
-        SnapshotContainsTypeTestingStubs(d->kind());
+    bool is_vm_object = d->isolate() == Dart::vm_isolate();
 
     for (intptr_t id = start_index_; id < stop_index_; id++) {
       RawTypeRef* type = reinterpret_cast<RawTypeRef*>(d->Ref(id));
@@ -3300,24 +3287,14 @@ class TypeRefDeserializationCluster : public DeserializationCluster {
       for (RawObject** p = from; p <= to; p++) {
         *p = d->ReadRef();
       }
-      if (should_read_type_testing_stub) {
+      if (d->kind() == Snapshot::kFullAOT) {
         instr_ = d->ReadInstructions();
         type_ = type;
         type_.SetTypeTestingStub(instr_);
       }
     }
   }
 
-  void PostLoad(const Array& refs, Snapshot::Kind kind, Zone* zone) {
-    if (!SnapshotContainsTypeTestingStubs(kind)) {
-      for (intptr_t id = start_index_; id < stop_index_; id++) {
-        type_ ^= refs.At(id);
-        instr_ = TypeTestingStubGenerator::DefaultCodeForType(type_);
-        type_.SetTypeTestingStub(instr_);
-      }
-    }
-  }
-
  private:
   AbstractType& type_;
   Instructions& instr_;
@@ -3354,9 +3331,6 @@ class TypeParameterSerializationCluster : public SerializationCluster {
   }
 
   void WriteFill(Serializer* s) {
-    const bool should_write_type_testing_stub =
-        SnapshotContainsTypeTestingStubs(s->kind());
-
     intptr_t count = objects_.length();
     for (intptr_t i = 0; i < count; i++) {
       RawTypeParameter* type = objects_[i];
@@ -3369,7 +3343,7 @@ class TypeParameterSerializationCluster : public SerializationCluster {
       s->WriteTokenPosition(type->ptr()->token_pos_);
       s->Write<int16_t>(type->ptr()->index_);
       s->Write<int8_t>(type->ptr()->type_state_);
-      if (should_write_type_testing_stub) {
+      if (s->kind() == Snapshot::kFullAOT) {
         RawInstructions* instr = type_testing_stubs_.LookupByAddresss(
             type->ptr()->type_test_stub_entry_point_);
         s->WriteInstructions(instr, Code::null());
@@ -3402,8 +3376,6 @@ class TypeParameterDeserializationCluster : public DeserializationCluster {
 
   void ReadFill(Deserializer* d) {
     bool is_vm_object = d->isolate() == Dart::vm_isolate();
-    const bool should_read_type_testing_stub =
-        SnapshotContainsTypeTestingStubs(d->kind());
 
     for (intptr_t id = start_index_; id < stop_index_; id++) {
       RawTypeParameter* type = reinterpret_cast<RawTypeParameter*>(d->Ref(id));
@@ -3418,7 +3390,7 @@ class TypeParameterDeserializationCluster : public DeserializationCluster {
       type->ptr()->token_pos_ = d->ReadTokenPosition();
       type->ptr()->index_ = d->Read<int16_t>();
       type->ptr()->type_state_ = d->Read<int8_t>();
-      if (should_read_type_testing_stub) {
+      if (d->kind() == Snapshot::kFullAOT) {
         instr_ = d->ReadInstructions();
         type_ = type;
         type_.SetTypeTestingStub(instr_);
@@ -3427,7 +3399,7 @@ class TypeParameterDeserializationCluster : public DeserializationCluster {
   }
 
   void PostLoad(const Array& refs, Snapshot::Kind kind, Zone* zone) {
-    if (!SnapshotContainsTypeTestingStubs(kind)) {
+    if (kind != Snapshot::kFullAOT) {
       for (intptr_t id = start_index_; id < stop_index_; id++) {
         type_ ^= refs.At(id);
         instr_ = TypeTestingStubGenerator::DefaultCodeForType(type_);

runtime/vm/compiler/backend/flow_graph_compiler.cc

@@ -1934,12 +1934,7 @@ void FlowGraphCompiler::GenerateCidRangesCheck(Assembler* assembler,
   }
 }
 
-bool FlowGraphCompiler::ShouldUseTypeTestingStubFor(bool optimizing,
-                                                    const AbstractType& type) {
-  return FLAG_precompiled_mode || (optimizing && type.IsTypeParameter());
-}
-
-void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
+void FlowGraphCompiler::GenerateAssertAssignableAOT(
     const AbstractType& dst_type,
     const String& dst_name,
     const Register instance_reg,

runtime/vm/compiler/backend/flow_graph_compiler.h

@@ -364,28 +364,21 @@ class FlowGraphCompiler : public ValueObject {
                                 const AbstractType& dst_type,
                                 const String& dst_name,
                                 LocationSummary* locs);
-
-  // Returns true if we can use a type testing stub based assert
-  // assignable code pattern for the given type.
-  static bool ShouldUseTypeTestingStubFor(bool optimizing,
-                                          const AbstractType& type);
-
-  void GenerateAssertAssignableViaTypeTestingStub(TokenPosition token_pos,
-                                                  intptr_t deopt_id,
-                                                  const AbstractType& dst_type,
-                                                  const String& dst_name,
-                                                  LocationSummary* locs);
-
-  void GenerateAssertAssignableViaTypeTestingStub(
-      const AbstractType& dst_type,
-      const String& dst_name,
-      const Register instance_reg,
-      const Register instantiator_type_args_reg,
-      const Register function_type_args_reg,
-      const Register subtype_cache_reg,
-      const Register dst_type_reg,
-      const Register scratch_reg,
-      Label* done);
+  void GenerateAssertAssignableAOT(TokenPosition token_pos,
+                                   intptr_t deopt_id,
+                                   const AbstractType& dst_type,
+                                   const String& dst_name,
+                                   LocationSummary* locs);
+
+  void GenerateAssertAssignableAOT(const AbstractType& dst_type,
+                                   const String& dst_name,
+                                   const Register instance_reg,
+                                   const Register instantiator_type_args_reg,
+                                   const Register function_type_args_reg,
+                                   const Register subtype_cache_reg,
+                                   const Register dst_type_reg,
+                                   const Register scratch_reg,
+                                   Label* done);
 
 // DBC emits calls very differently from all other architectures due to its
 // interpreted nature.

runtime/vm/compiler/backend/flow_graph_compiler_arm.cc

@@ -663,8 +663,7 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
   }
 
   if (FLAG_precompiled_mode) {
-    GenerateAssertAssignableViaTypeTestingStub(token_pos, deopt_id, dst_type,
-                                               dst_name, locs);
+    GenerateAssertAssignableAOT(token_pos, deopt_id, dst_type, dst_name, locs);
   } else {
     Label is_assignable_fast, is_assignable, runtime_call;
 
@@ -692,11 +691,10 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
     __ PushObject(dst_name);  // Push the name of the destination.
     __ LoadUniqueObject(R0, test_cache);
     __ Push(R0);
-    __ PushObject(Smi::ZoneHandle(zone(), Smi::New(kTypeCheckFromInline)));
-    GenerateRuntimeCall(token_pos, deopt_id, kTypeCheckRuntimeEntry, 7, locs);
+    GenerateRuntimeCall(token_pos, deopt_id, kTypeCheckRuntimeEntry, 6, locs);
     // Pop the parameters supplied to the runtime entry. The result of the
     // type check runtime call is the checked value.
-    __ Drop(7);
+    __ Drop(6);
     __ Pop(R0);
     __ Bind(&is_assignable);
     __ PopList((1 << kFunctionTypeArgumentsReg) |
@@ -705,7 +703,7 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
   }
 }
 
-void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
+void FlowGraphCompiler::GenerateAssertAssignableAOT(
     TokenPosition token_pos,
     intptr_t deopt_id,
     const AbstractType& dst_type,
@@ -721,10 +719,10 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
 
   Label done;
 
-  GenerateAssertAssignableViaTypeTestingStub(
-      dst_type, dst_name, kInstanceReg, kInstantiatorTypeArgumentsReg,
-      kFunctionTypeArgumentsReg, kSubtypeTestCacheReg, kDstTypeReg, kScratchReg,
-      &done);
+  GenerateAssertAssignableAOT(dst_type, dst_name, kInstanceReg,
+                              kInstantiatorTypeArgumentsReg,
+                              kFunctionTypeArgumentsReg, kSubtypeTestCacheReg,
+                              kDstTypeReg, kScratchReg, &done);
   // We use 2 consecutive entries in the pool for the subtype cache and the
   // destination name.  The second entry, namely [dst_name] seems to be unused,
   // but it will be used by the code throwing a TypeError if the type test fails

runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc

@@ -643,9 +643,8 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
     return;
   }
 
-  if (ShouldUseTypeTestingStubFor(is_optimizing(), dst_type)) {
-    GenerateAssertAssignableViaTypeTestingStub(token_pos, deopt_id, dst_type,
-                                               dst_name, locs);
+  if (FLAG_precompiled_mode) {
+    GenerateAssertAssignableAOT(token_pos, deopt_id, dst_type, dst_name, locs);
   } else {
     Label is_assignable_fast, is_assignable, runtime_call;
 
@@ -670,19 +669,18 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
     __ PushObject(dst_name);  // Push the name of the destination.
     __ LoadUniqueObject(R0, test_cache);
     __ Push(R0);
-    __ PushObject(Smi::ZoneHandle(zone(), Smi::New(kTypeCheckFromInline)));
-    GenerateRuntimeCall(token_pos, deopt_id, kTypeCheckRuntimeEntry, 7, locs);
+    GenerateRuntimeCall(token_pos, deopt_id, kTypeCheckRuntimeEntry, 6, locs);
     // Pop the parameters supplied to the runtime entry. The result of the
     // type check runtime call is the checked value.
-    __ Drop(7);
+    __ Drop(6);
     __ Pop(R0);
     __ Bind(&is_assignable);
     __ PopPair(kFunctionTypeArgumentsReg, kInstantiatorTypeArgumentsReg);
     __ Bind(&is_assignable_fast);
   }
 }
 
-void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
+void FlowGraphCompiler::GenerateAssertAssignableAOT(
     TokenPosition token_pos,
     intptr_t deopt_id,
     const AbstractType& dst_type,
@@ -698,10 +696,10 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
 
   Label done;
 
-  GenerateAssertAssignableViaTypeTestingStub(
-      dst_type, dst_name, kInstanceReg, kInstantiatorTypeArgumentsReg,
-      kFunctionTypeArgumentsReg, kSubtypeTestCacheReg, kDstTypeReg, kScratchReg,
-      &done);
+  GenerateAssertAssignableAOT(dst_type, dst_name, kInstanceReg,
+                              kInstantiatorTypeArgumentsReg,
+                              kFunctionTypeArgumentsReg, kSubtypeTestCacheReg,
+                              kDstTypeReg, kScratchReg, &done);
 
   // We use 2 consecutive entries in the pool for the subtype cache and the
   // destination name.  The second entry, namely [dst_name] seems to be unused,

runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc

@@ -686,11 +686,10 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
   __ PushObject(dst_name);  // Push the name of the destination.
   __ LoadObject(EAX, test_cache);
   __ pushl(EAX);
-  __ PushObject(Smi::ZoneHandle(zone(), Smi::New(kTypeCheckFromInline)));
-  GenerateRuntimeCall(token_pos, deopt_id, kTypeCheckRuntimeEntry, 7, locs);
+  GenerateRuntimeCall(token_pos, deopt_id, kTypeCheckRuntimeEntry, 6, locs);
   // Pop the parameters supplied to the runtime entry. The result of the
   // type check runtime call is the checked value.
-  __ Drop(7);
+  __ Drop(6);
   __ popl(EAX);
 
   __ Bind(&is_assignable);