[vm/compiler] Ensure TTS stubs can handle types where the tav is at l…

…arge offset If the type arguments vector gets introduced in subclasses where the base classes have already many fields we may not be able to load the TAV in one instruction on ARM64. Issue flutter/flutter#82278 TEST=vm/dart{,_2}/flutter_regress_82278_test Change-Id: I164ef42af3afe8267fe23a8a11af9401776eccdb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199481 Commit-Queue: Martin Kustermann <kustermann@google.com> Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
dart-lang · May 17, 2021 · aa91e8c · aa91e8c
1 parent 9b16765
commit aa91e8c
Show file tree

Hide file tree

Showing 8 changed files with 142 additions and 10 deletions.
diff --git a/runtime/tests/vm/dart/flutter_regress_82278_test.dart b/runtime/tests/vm/dart/flutter_regress_82278_test.dart
@@ -0,0 +1,62 @@
+// Copyright (c) 2021, 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';
+
+class Foo {
+  static int _next = 0;
+
+  var field0 = ++_next;
+  var field1 = ++_next;
+  var field2 = ++_next;
+  var field3 = ++_next;
+  var field4 = ++_next;
+  var field5 = ++_next;
+  var field6 = ++_next;
+  var field7 = ++_next;
+  var field8 = ++_next;
+  var field9 = ++_next;
+  var field10 = ++_next;
+  var field11 = ++_next;
+  var field12 = ++_next;
+  var field13 = ++_next;
+  var field14 = ++_next;
+  var field15 = ++_next;
+  var field16 = ++_next;
+  var field17 = ++_next;
+  var field18 = ++_next;
+  var field19 = ++_next;
+  var field20 = ++_next;
+  var field21 = ++_next;
+  var field22 = ++_next;
+  var field23 = ++_next;
+  var field24 = ++_next;
+  var field25 = ++_next;
+  var field26 = ++_next;
+  var field27 = ++_next;
+  var field28 = ++_next;
+  var field29 = ++_next;
+  var field30 = ++_next;
+  var field31 = ++_next;
+
+  @pragma('vm:never-inline')
+  String toString() => '$field0 $field1 $field2 $field3 $field4 $field5'
+      '$field6 $field7 $field8 $field9 $field10 $field11'
+      '$field12 $field13 $field14 $field15 $field16 $field17'
+      '$field18 $field19 $field20 $field21 $field22 $field23'
+      '$field24 $field25 $field26 $field27 $field28 $field29'
+      '$field30 $field3';
+}
+
+// The TypeArgumentVector will be at an offset that cannot be loaded from via
+// normal addresing mode on ARM64.
+class GenericFoo<T> extends Foo {}
+
+final l = <dynamic>[GenericFoo<int>(), null, 1, 1.0];
+
+main() {
+  final dynamic genericFoo = l[0];
+  Expect.isTrue(genericFoo.toString().length > 0); // Keep the fields alive.
+  Expect.isTrue(identical(genericFoo as GenericFoo<int>, genericFoo));
+}
diff --git a/runtime/tests/vm/dart_2/flutter_regress_82278_test.dart b/runtime/tests/vm/dart_2/flutter_regress_82278_test.dart
@@ -0,0 +1,62 @@
+// Copyright (c) 2021, 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';
+
+class Foo {
+  static int _next = 0;
+
+  var field0 = ++_next;
+  var field1 = ++_next;
+  var field2 = ++_next;
+  var field3 = ++_next;
+  var field4 = ++_next;
+  var field5 = ++_next;
+  var field6 = ++_next;
+  var field7 = ++_next;
+  var field8 = ++_next;
+  var field9 = ++_next;
+  var field10 = ++_next;
+  var field11 = ++_next;
+  var field12 = ++_next;
+  var field13 = ++_next;
+  var field14 = ++_next;
+  var field15 = ++_next;
+  var field16 = ++_next;
+  var field17 = ++_next;
+  var field18 = ++_next;
+  var field19 = ++_next;
+  var field20 = ++_next;
+  var field21 = ++_next;
+  var field22 = ++_next;
+  var field23 = ++_next;
+  var field24 = ++_next;
+  var field25 = ++_next;
+  var field26 = ++_next;
+  var field27 = ++_next;
+  var field28 = ++_next;
+  var field29 = ++_next;
+  var field30 = ++_next;
+  var field31 = ++_next;
+
+  @pragma('vm:never-inline')
+  String toString() => '$field0 $field1 $field2 $field3 $field4 $field5'
+      '$field6 $field7 $field8 $field9 $field10 $field11'
+      '$field12 $field13 $field14 $field15 $field16 $field17'
+      '$field18 $field19 $field20 $field21 $field22 $field23'
+      '$field24 $field25 $field26 $field27 $field28 $field29'
+      '$field30 $field3';
+}
+
+// The TypeArgumentVector will be at an offset that cannot be loaded from via
+// normal addresing mode on ARM64.
+class GenericFoo<T> extends Foo {}
+
+final l = <dynamic>[GenericFoo<int>(), null, 1, 1.0];
+
+main() {
+  final dynamic genericFoo = l[0];
+  Expect.isTrue(genericFoo.toString().length > 0); // Keep the fields alive.
+  Expect.isTrue(identical(genericFoo as GenericFoo<int>, genericFoo));
+}
diff --git a/runtime/vm/compiler/assembler/assembler_arm.h b/runtime/vm/compiler/assembler/assembler_arm.h
@@ -941,8 +941,14 @@ class Assembler : public AssemblerBase {
   void LoadFieldFromOffset(Register reg,
                            Register base,
                            int32_t offset,
-                           OperandSize type = kFourBytes,
-                           Condition cond = AL) {
+                           OperandSize sz = kFourBytes) override {
+    LoadFieldFromOffset(reg, base, offset, sz, AL);
+  }
+  void LoadFieldFromOffset(Register reg,
+                           Register base,
+                           int32_t offset,
+                           OperandSize type,
+                           Condition cond) {
     LoadFromOffset(reg, base, offset - kHeapObjectTag, type, cond);
   }
   void LoadCompressedFieldFromOffset(Register reg,

diff --git a/runtime/vm/compiler/assembler/assembler_arm64.h b/runtime/vm/compiler/assembler/assembler_arm64.h
@@ -1697,7 +1697,7 @@ class Assembler : public AssemblerBase {
   void LoadFieldFromOffset(Register dest,
                            Register base,
                            int32_t offset,
-                           OperandSize sz = kEightBytes) {
+                           OperandSize sz = kEightBytes) override {
     LoadFromOffset(dest, base, offset - kHeapObjectTag, sz);
   }
   void LoadCompressedFieldFromOffset(Register dest,

diff --git a/runtime/vm/compiler/assembler/assembler_base.h b/runtime/vm/compiler/assembler/assembler_base.h
@@ -605,6 +605,10 @@ class AssemblerBase : public StackResource {
   };
 
   virtual void LoadField(Register dst, const FieldAddress& address) = 0;
+  virtual void LoadFieldFromOffset(Register reg,
+                                   Register base,
+                                   int32_t offset,
+                                   OperandSize = kWordBytes) = 0;
   void LoadFromSlot(Register dst, Register base, const Slot& slot);
 
   virtual void StoreIntoObject(

diff --git a/runtime/vm/compiler/assembler/assembler_ia32.h b/runtime/vm/compiler/assembler/assembler_ia32.h
@@ -607,7 +607,7 @@ class Assembler : public AssemblerBase {
   void LoadFieldFromOffset(Register reg,
                            Register base,
                            int32_t offset,
-                           OperandSize sz = kFourBytes) {
+                           OperandSize sz = kFourBytes) override {
     LoadFromOffset(reg, FieldAddress(base, offset), sz);
   }
   void LoadCompressedFieldFromOffset(Register reg,

diff --git a/runtime/vm/compiler/assembler/assembler_x64.h b/runtime/vm/compiler/assembler/assembler_x64.h
@@ -971,7 +971,7 @@ class Assembler : public AssemblerBase {
   void LoadFieldFromOffset(Register dst,
                            Register base,
                            int32_t offset,
-                           OperandSize sz = kEightBytes) {
+                           OperandSize sz = kEightBytes) override {
     LoadFromOffset(dst, FieldAddress(base, offset), sz);
   }
   void LoadCompressedFieldFromOffset(Register dst,

diff --git a/runtime/vm/type_testing_stubs.cc b/runtime/vm/type_testing_stubs.cc
@@ -415,11 +415,9 @@ void TypeTestingStubGenerator::
   // fall through to continue
 
   // b) Then we'll load the values for the type parameters.
-  __ LoadField(
-      TTSInternalRegs::kInstanceTypeArgumentsReg,
-      compiler::FieldAddress(
-          TypeTestABI::kInstanceReg,
-          compiler::target::Class::TypeArgumentsFieldOffset(type_class)));
+  __ LoadFieldFromOffset(
+      TTSInternalRegs::kInstanceTypeArgumentsReg, TypeTestABI::kInstanceReg,
+      compiler::target::Class::TypeArgumentsFieldOffset(type_class));
 
   // The kernel frontend should fill in any non-assigned type parameters on
   // construction with dynamic/Object, so we should never get the null type