[vm/compiler] Fix TypeTestingStub -> SubtypeTestCache fallback code i…

…f dst_type = TypeParameter

If the dst_type of an AssertAssignable is a type parameter, the AssertAssignable
implementation will load the value of the type parameter using
the (instantiator or function) type arguments. It will then call the
type testing stub (TTS) of that type.

If the TTS is not exhaustive (e.g. because `T = X<..>` wher `X` is
implemented), it can fall back to the slower SubTypeTestCache implementation.

Right now the STC fallback will get the loaded value of the type
parameter for `dst_type` instead of the type parameter. Doing so is
incorrect.

  => This CL ensures we preserve dst_type = TypeParameter for the STC
  fallback.

Issue #39716

Change-Id: Idea2405efbdc01c031ee68dbb345820e721533eb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/127640
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Régis Crelier <regis@google.com>

runtime/tests/vm/dart/tts_regression39716_test.dart

@@ -0,0 +1,50 @@
+// Copyright (c) 2019, 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.
+
+import 'package:expect/expect.dart';
+
+main() {
+  // Ensure all classes are used.
+  final l = <dynamic>[Base(), A<int>(), B<int>(), I<int>()];
+  print(l);
+
+  // Call `add(T)` with `T = A<int/String>` (the VM will not generate an
+  // optimized TTS for `A<int>` because there's two classes implementing
+  // `A`, both have type argument vector at a different offset, so it wouldn't
+  // know where to load it from)
+
+  // Populate the SubtypeTestCache with successfull `add(I<String>())`.
+  final x = <dynamic>[<A<String>>[]];
+  x.single.add(I<String>());
+
+  // Ensure type check fails if list type is now `A<int>`.
+  final y = <dynamic>[<A<int>>[]];
+  String exception = '';
+  try {
+    y.single.add(I<String>());
+  } catch (e) {
+    exception = e.toString();
+  }
+  print(exception);
+  Expect.isTrue(exception.contains('is not a subtype of'));
+}
+
+// Ensure depth-first preorder cid numbering will have the cid for [A] in the
+// middle and ensure type argument vector will be second field.
+class Base {
+  final int baseField = int.parse('1');
+}
+
+class A<T> extends Base {
+  T foo(T arg) => arg;
+}
+
+class B<T> extends A<T> {}
+
+// Make another implementation of A<T> where type argument vector is first
+// field.
+class I<T> implements A<T> {
+  int get baseField => int.parse('2');
+  T foo(T arg) => arg;
+}

runtime/vm/compiler/backend/flow_graph_compiler.cc

@@ -2237,6 +2237,7 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
     const Register function_type_args_reg,
     const Register subtype_cache_reg,
     const Register dst_type_reg,
+    const Register dst_type_reg_to_call,
     const Register scratch_reg,
     compiler::Label* done) {
   TypeUsageInfo* type_usage_info = thread()->type_usage_info();
@@ -2250,6 +2251,11 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
     is_non_smi = true;
   }
 
+  // We use two type registers iff the dst type is a type parameter.
+  // We "dereference" the type parameter for the TTS call but leave the type
+  // parameter in the dst_type_reg for fallback into SubtypeTestCache.
+  ASSERT(dst_type.IsTypeParameter() == (dst_type_reg != dst_type_reg_to_call));
+
   // We can handle certain types very efficiently on the call site (with a
   // bailout to the normal stub, which will do a runtime call).
   if (dst_type.IsTypeParameter()) {
@@ -2262,10 +2268,11 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
     __ CompareObject(kTypeArgumentsReg, Object::null_object());
     __ BranchIf(EQUAL, done);
     __ LoadField(
-        dst_type_reg,
+        dst_type_reg_to_call,
         compiler::FieldAddress(kTypeArgumentsReg,
                                compiler::target::TypeArguments::type_at_offset(
                                    type_param.index())));
+    __ LoadObject(dst_type_reg, type_param);
     if (type_usage_info != NULL) {
       type_usage_info->UseTypeInAssertAssignable(dst_type);
     }

runtime/vm/compiler/backend/flow_graph_compiler.h

@@ -501,6 +501,7 @@ class FlowGraphCompiler : public ValueObject {
       const Register function_type_args_reg,
       const Register subtype_cache_reg,
       const Register dst_type_reg,
+      const Register dst_type_reg_to_call,
       const Register scratch_reg,
       compiler::Label* done);
 

runtime/vm/compiler/backend/flow_graph_compiler_arm.cc

@@ -762,14 +762,15 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
 
   const Register kSubtypeTestCacheReg = R3;
   const Register kDstTypeReg = R8;
+  const Register kRegToCall = dst_type.IsTypeParameter() ? R9 : kDstTypeReg;
   const Register kScratchReg = R4;
 
   compiler::Label done;
 
   GenerateAssertAssignableViaTypeTestingStub(
       dst_type, dst_name, kInstanceReg, kInstantiatorTypeArgumentsReg,
-      kFunctionTypeArgumentsReg, kSubtypeTestCacheReg, kDstTypeReg, kScratchReg,
-      &done);
+      kFunctionTypeArgumentsReg, kSubtypeTestCacheReg, kDstTypeReg, kRegToCall,
+      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
@@ -789,7 +790,7 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
   __ LoadField(
       R9,
       compiler::FieldAddress(
-          kDstTypeReg,
+          kRegToCall,
           compiler::target::AbstractType::type_test_stub_entry_point_offset()));
   __ LoadWordFromPoolOffset(kSubtypeTestCacheReg, sub_type_cache_offset, PP,
                             AL);

runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc

@@ -737,14 +737,15 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
 
   const Register kSubtypeTestCacheReg = R3;
   const Register kDstTypeReg = R8;
+  const Register kRegToCall = dst_type.IsTypeParameter() ? R9 : kDstTypeReg;
   const Register kScratchReg = R4;
 
   compiler::Label done;
 
   GenerateAssertAssignableViaTypeTestingStub(
       dst_type, dst_name, kInstanceReg, kInstantiatorTypeArgumentsReg,
-      kFunctionTypeArgumentsReg, kSubtypeTestCacheReg, kDstTypeReg, kScratchReg,
-      &done);
+      kFunctionTypeArgumentsReg, kSubtypeTestCacheReg, kDstTypeReg, kRegToCall,
+      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,
@@ -763,7 +764,7 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
 
   __ LoadField(
       R9, compiler::FieldAddress(
-              kDstTypeReg, AbstractType::type_test_stub_entry_point_offset()));
+              kRegToCall, AbstractType::type_test_stub_entry_point_offset()));
   __ LoadWordFromPoolOffset(kSubtypeTestCacheReg, sub_type_cache_offset);
   __ blr(R9);
   EmitCallsiteMetadata(token_pos, deopt_id, RawPcDescriptors::kOther, locs);

runtime/vm/compiler/backend/flow_graph_compiler_x64.cc

@@ -750,12 +750,14 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
   compiler::Label done;
 
   const Register subtype_cache_reg = R9;
-  const Register kScratchReg = RBX;
+  const Register kDstTypeReg = RBX;
+  const Register kRegToCall = dst_type.IsTypeParameter() ? RSI : kDstTypeReg;
+  const Register kScratchReg = kRegToCall;
 
   GenerateAssertAssignableViaTypeTestingStub(
       dst_type, dst_name, kInstanceReg, kInstantiatorTypeArgumentsReg,
-      kFunctionTypeArgumentsReg, subtype_cache_reg, kScratchReg, kScratchReg,
-      &done);
+      kFunctionTypeArgumentsReg, subtype_cache_reg, kDstTypeReg, kRegToCall,
+      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,
@@ -774,7 +776,7 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
 
   __ movq(subtype_cache_reg, compiler::Address(PP, sub_type_cache_offset));
   __ call(compiler::FieldAddress(
-      RBX, AbstractType::type_test_stub_entry_point_offset()));
+      kRegToCall, AbstractType::type_test_stub_entry_point_offset()));
   EmitCallsiteMetadata(token_pos, deopt_id, RawPcDescriptors::kOther, locs);
   __ Bind(&done);
 }

runtime/vm/compiler/stub_code_compiler_arm.cc

@@ -2798,18 +2798,6 @@ void StubCodeCompiler::GenerateTopTypeTypeTestStub(Assembler* assembler) {
   __ Ret();
 }
 
-void StubCodeCompiler::GenerateTypeRefTypeTestStub(Assembler* assembler) {
-  const Register kTypeRefReg = R8;
-
-  // We dereference the TypeRef and tail-call to it's type testing stub.
-  __ ldr(kTypeRefReg,
-         FieldAddress(kTypeRefReg, target::TypeRef::type_offset()));
-  __ ldr(R9, FieldAddress(
-                 kTypeRefReg,
-                 target::AbstractType::type_test_stub_entry_point_offset()));
-  __ bx(R9);
-}
-
 void StubCodeCompiler::GenerateUnreachableTypeTestStub(Assembler* assembler) {
   __ Breakpoint();
 }

runtime/vm/compiler/stub_code_compiler_arm64.cc

@@ -2890,18 +2890,6 @@ void StubCodeCompiler::GenerateTopTypeTypeTestStub(Assembler* assembler) {
   __ Ret();
 }
 
-void StubCodeCompiler::GenerateTypeRefTypeTestStub(Assembler* assembler) {
-  const Register kTypeRefReg = R8;
-
-  // We dereference the TypeRef and tail-call to it's type testing stub.
-  __ ldr(kTypeRefReg,
-         FieldAddress(kTypeRefReg, target::TypeRef::type_offset()));
-  __ ldr(R9, FieldAddress(
-                 kTypeRefReg,
-                 target::AbstractType::type_test_stub_entry_point_offset()));
-  __ br(R9);
-}
-
 void StubCodeCompiler::GenerateUnreachableTypeTestStub(Assembler* assembler) {
   __ Breakpoint();
 }

runtime/vm/compiler/stub_code_compiler_ia32.cc

@@ -2432,11 +2432,6 @@ void StubCodeCompiler::GenerateTopTypeTypeTestStub(Assembler* assembler) {
   __ Breakpoint();
 }
 
-void StubCodeCompiler::GenerateTypeRefTypeTestStub(Assembler* assembler) {
-  // Not implemented on ia32.
-  __ Breakpoint();
-}
-
 void StubCodeCompiler::GenerateUnreachableTypeTestStub(Assembler* assembler) {
   // Not implemented on ia32.
   __ Breakpoint();

runtime/vm/compiler/stub_code_compiler_x64.cc

@@ -2927,16 +2927,6 @@ void StubCodeCompiler::GenerateTopTypeTypeTestStub(Assembler* assembler) {
   __ Ret();
 }
 
-void StubCodeCompiler::GenerateTypeRefTypeTestStub(Assembler* assembler) {
-  const Register kTypeRefReg = RBX;
-
-  // We dereference the TypeRef and tail-call to it's type testing stub.
-  __ movq(kTypeRefReg,
-          FieldAddress(kTypeRefReg, target::TypeRef::type_offset()));
-  __ jmp(FieldAddress(
-      kTypeRefReg, target::AbstractType::type_test_stub_entry_point_offset()));
-}
-
 void StubCodeCompiler::GenerateUnreachableTypeTestStub(Assembler* assembler) {
   __ Breakpoint();
 }

runtime/vm/instructions_x64.cc

@@ -81,12 +81,12 @@ bool DecodeLoadObjectFromPoolOrThread(uword pc, const Code& code, Object* obj) {
 
 intptr_t TypeTestingStubCallPattern::GetSubtypeTestCachePoolIndex() {
   static int16_t pattern_disp8[] = {
-      0x4d, 0x8b, 0x4f, -1,  // movq R9, [PP + offset]
-      0xff, 0x53, 0x07,      // callq [RBX + 0x7]
+      0x4d, 0x8b, 0x4f, -1,               // movq R9, [PP + offset]
+      0xff, -1 /* 0x53 or 0x56 */, 0x07,  // callq [RBX/RSI + 0x7]
   };
   static int16_t pattern_disp32[] = {
-      0x4d, 0x8b, 0x8f, -1, -1, -1, -1,  // movq R9, [PP + offset]
-      0xff, 0x53, 0x07,                  // callq [RBX + 0x7]
+      0x4d, 0x8b, 0x8f, -1, -1, -1, -1,   // movq R9, [PP + offset]
+      0xff, -1 /* 0x53 or 0x56 */, 0x07,  // callq [RBX/RSI + 0x7]
   };
   if (MatchesPattern(pc_, pattern_disp8, ARRAY_SIZE(pattern_disp8))) {
     return IndexFromPPLoadDisp8(pc_ - 4);

runtime/vm/stub_code_list.h

@@ -62,7 +62,6 @@ namespace dart {
   V(Subtype6TestCache)                                                         \
   V(DefaultTypeTest)                                                           \
   V(TopTypeTypeTest)                                                           \
-  V(TypeRefTypeTest)                                                           \
   V(UnreachableTypeTest)                                                       \
   V(SlowTypeTest)                                                              \
   V(LazySpecializeTypeTest)                                                    \

runtime/vm/type_testing_stubs.cc

@@ -70,10 +70,6 @@ const char* TypeTestingStubNamer::StringifyType(
   } else if (type.IsTypeParameter()) {
     string_ = TypeParameter::Cast(type).name();
     return AssemblerSafeName(OS::SCreate(Z, "%s", string_.ToCString()));
-  } else if (type.IsTypeRef()) {
-    const Type& dereferenced_type =
-        Type::Handle(Type::RawCast(TypeRef::Cast(type).type()));
-    return OS::SCreate(Z, "TypeRef_%s", StringifyType(dereferenced_type));
   } else {
     return AssemblerSafeName(OS::SCreate(Z, "%s", type.ToCString()));
   }
@@ -96,7 +92,7 @@ RawCode* TypeTestingStubGenerator::DefaultCodeForType(
     const AbstractType& type,
     bool lazy_specialize /* = true */) {
   if (type.IsTypeRef()) {
-    return StubCode::TypeRefTypeTest().raw();
+    return StubCode::DefaultTypeTest().raw();
   }
 
   const intptr_t cid = type.type_class_id();
@@ -147,7 +143,7 @@ RawCode* TypeTestingStubGenerator::OptimizedCodeForType(
   ASSERT(StubCode::HasBeenInitialized());
 
   if (type.IsTypeRef()) {
-    return StubCode::TypeRefTypeTest().raw();
+    return StubCode::DefaultTypeTest().raw();
   }
 
   const intptr_t cid = type.type_class_id();