[vm/aot] Unbox records using inferred record shapes

Unboxing of records in return values is switched to use inferred record
shape instead of a static type. This is more accurate, as
static return type 'Object', 'dynamic' or a type parameter would
not prevent record unboxing.

TEST=runtime/tests/vm/dart/records_return_value_unboxing_il_test.dart
TEST=pkg/vm/testcases/transformations/type_flow/transformer/unboxed_records.dart

Issue: #49719
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-precomp-nnbd-linux-release-x64-try
Change-Id: I56528e114bc2de94c4a1ec09c48eb5b9ed3c3d3f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/288824
Commit-Queue: Alexander Markov <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>

Author: alexmarkov

pkg/vm/lib/metadata/unboxing_info.dart

@@ -3,82 +3,139 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:kernel/ast.dart';
+import 'package:vm/transformations/type_flow/types.dart' show RecordShape;
 
-class UnboxingInfoMetadata {
-  static const kBoxed = 0;
-  static const kUnboxedIntCandidate = 1 << 0;
-  static const kUnboxedDoubleCandidate = 1 << 1;
-  static const kUnboxedRecordCandidate = 1 << 2;
-  static const kUnboxingCandidate =
-      kUnboxedIntCandidate | kUnboxedDoubleCandidate | kUnboxedRecordCandidate;
+enum UnboxingKind {
+  boxed,
+  int,
+  double,
+  record,
+  unknown, // Not calculated yet.
+}
 
-  final List<int> unboxedArgsInfo;
-  int returnInfo;
+class UnboxingType {
+  final UnboxingKind kind;
+  final RecordShape? recordShape;
 
-  UnboxingInfoMetadata(int argsLen)
-      : unboxedArgsInfo = [],
-        returnInfo = kUnboxingCandidate {
-    for (int i = 0; i < argsLen; i++) {
-      unboxedArgsInfo.add(kUnboxingCandidate);
+  const UnboxingType._(this.kind, this.recordShape);
+  UnboxingType.record(RecordShape shape) : this._(UnboxingKind.record, shape);
+
+  static const kUnknown = UnboxingType._(UnboxingKind.unknown, null);
+  static const kInt = UnboxingType._(UnboxingKind.int, null);
+  static const kDouble = UnboxingType._(UnboxingKind.double, null);
+  static const kBoxed = UnboxingType._(UnboxingKind.boxed, null);
+
+  UnboxingType intersect(UnboxingType other) {
+    if (kind == UnboxingKind.unknown) return other;
+    if (other.kind == UnboxingKind.unknown) return this;
+    if (this == other) return this;
+    return kBoxed;
+  }
+
+  @override
+  bool operator ==(other) =>
+      identical(this, other) ||
+      (other is UnboxingType &&
+          this.kind == other.kind &&
+          this.recordShape == other.recordShape);
+
+  @override
+  int get hashCode => (kind.index * 31) + recordShape.hashCode;
+
+  void writeToBinary(BinarySink sink) {
+    sink.writeUInt30(kind.index);
+    if (kind == UnboxingKind.record) {
+      recordShape!.writeToBinary(sink);
+    }
+  }
+
+  factory UnboxingType.readFromBinary(BinarySource source) {
+    final kind = UnboxingKind.values[source.readUInt30()];
+    final recordShape = (kind == UnboxingKind.record)
+        ? RecordShape.readFromBinary(source)
+        : null;
+    return UnboxingType._(kind, recordShape);
+  }
+
+  @override
+  String toString() {
+    switch (kind) {
+      case UnboxingKind.boxed:
+        return 'b';
+      case UnboxingKind.int:
+        return 'i';
+      case UnboxingKind.double:
+        return 'd';
+      case UnboxingKind.record:
+        {
+          final sb = StringBuffer();
+          sb.write('r<');
+          sb.write(recordShape!.numPositionalFields.toString());
+          for (final named in recordShape!.namedFields) {
+            sb.write(',');
+            sb.write(named);
+          }
+          sb.write('>');
+          return sb.toString();
+        }
+      case UnboxingKind.unknown:
+        throw 'Unexpected UnboxingType.kUknown';
     }
   }
+}
+
+class UnboxingInfoMetadata {
+  final List<UnboxingType> argsInfo;
+  UnboxingType returnInfo;
+
+  UnboxingInfoMetadata(int argsLen)
+      : argsInfo = List<UnboxingType>.filled(argsLen, UnboxingType.kUnknown,
+            growable: true),
+        returnInfo = UnboxingType.kUnknown;
 
   UnboxingInfoMetadata.readFromBinary(BinarySource source)
-      : unboxedArgsInfo = List<int>.generate(
-            source.readUInt30(), (_) => source.readByte(),
+      : argsInfo = List<UnboxingType>.generate(
+            source.readUInt30(), (_) => UnboxingType.readFromBinary(source),
             growable: true),
-        returnInfo = source.readByte();
+        returnInfo = UnboxingType.readFromBinary(source);
 
   // Returns `true` if all arguments as well as the return value have to be
   // boxed.
   //
   // We don't have to write out metadata for fully boxed methods, because this
   // is the default.
   bool get isFullyBoxed {
-    if (returnInfo != kBoxed) return false;
-    for (int argInfo in unboxedArgsInfo) {
-      if (argInfo != kBoxed) return false;
+    if (returnInfo != UnboxingType.kBoxed) return false;
+    for (final argInfo in argsInfo) {
+      if (argInfo != UnboxingType.kBoxed) return false;
     }
     return true;
   }
 
   void writeToBinary(BinarySink sink) {
-    sink.writeUInt30(unboxedArgsInfo.length);
-    for (int val in unboxedArgsInfo) {
-      sink.writeByte(val);
+    sink.writeUInt30(argsInfo.length);
+    for (final argInfo in argsInfo) {
+      argInfo.writeToBinary(sink);
     }
-    sink.writeByte(returnInfo);
+    returnInfo.writeToBinary(sink);
   }
 
   @override
   String toString() {
     final sb = StringBuffer();
     sb.write('(');
-    for (int i = 0; i < unboxedArgsInfo.length; ++i) {
-      final argInfo = unboxedArgsInfo[i];
-      sb.write(_stringifyUnboxingInfo(argInfo));
-      if (i != (unboxedArgsInfo.length - 1)) {
+    for (int i = 0; i < argsInfo.length; ++i) {
+      final argInfo = argsInfo[i];
+      sb.write(argInfo.toString());
+      if (i != (argsInfo.length - 1)) {
         sb.write(',');
       }
     }
     sb.write(')');
     sb.write('->');
-    sb.write(_stringifyUnboxingInfo(returnInfo));
+    sb.write(returnInfo.toString());
     return sb.toString();
   }
-
-  static String _stringifyUnboxingInfo(int info) {
-    switch (info) {
-      case UnboxingInfoMetadata.kUnboxedIntCandidate:
-        return 'i';
-      case UnboxingInfoMetadata.kUnboxedDoubleCandidate:
-        return 'd';
-      case UnboxingInfoMetadata.kUnboxedRecordCandidate:
-        return 'r';
-    }
-    assert(info == UnboxingInfoMetadata.kBoxed);
-    return 'b';
-  }
 }
 
 class UnboxingInfoMetadataRepository

pkg/vm/lib/transformations/type_flow/analysis.dart

@@ -1006,16 +1006,15 @@ class _TFClassImpl extends TFClass {
   late final Map<Name, Member> _dispatchTargetsNonSetters =
       _initDispatchTargets(false);
   final _DependencyTracker dependencyTracker = new _DependencyTracker();
-  final RecordShape? recordShape;
 
   /// Flag indicating if this class has a noSuchMethod() method not inherited
   /// from Object.
   /// Lazy initialized by ClassHierarchyCache.hasNonTrivialNoSuchMethod().
   bool? hasNonTrivialNoSuchMethod;
 
   _TFClassImpl(int id, Class classNode, this.superclass, this.supertypes,
-      this.recordShape)
-      : super(id, classNode) {
+      RecordShape? recordShape)
+      : super(id, classNode, recordShape) {
     supertypes.add(this);
   }
 

pkg/vm/lib/transformations/type_flow/rta.dart

@@ -55,7 +55,7 @@ class ClassInfo extends TFClass {
 
   ClassInfo(int id, Class classNode, this.superclass, this.supertypes,
       this.calledDynamicSelectors, this.calledVirtualSelectors)
-      : super(id, classNode) {
+      : super(id, classNode, null) {
     supertypes.add(this);
     for (var sup in supertypes) {
       sup.subtypes.add(this);

pkg/vm/lib/transformations/type_flow/transformer.dart

@@ -519,17 +519,12 @@ class AnnotateKernel extends RecursiveVisitor {
             _unboxingInfo.getUnboxingInfoOfMember(member);
         if (unboxingInfoMetadata != null) {
           // Check for partitions that only have abstract methods should be marked as boxed.
-          if (unboxingInfoMetadata.returnInfo ==
-              UnboxingInfoMetadata.kUnboxingCandidate) {
-            unboxingInfoMetadata.returnInfo = UnboxingInfoMetadata.kBoxed;
+          if (unboxingInfoMetadata.returnInfo == UnboxingType.kUnknown) {
+            unboxingInfoMetadata.returnInfo = UnboxingType.kBoxed;
           }
-          for (int i = 0;
-              i < unboxingInfoMetadata.unboxedArgsInfo.length;
-              i++) {
-            if (unboxingInfoMetadata.unboxedArgsInfo[i] ==
-                UnboxingInfoMetadata.kUnboxingCandidate) {
-              unboxingInfoMetadata.unboxedArgsInfo[i] =
-                  UnboxingInfoMetadata.kBoxed;
+          for (int i = 0; i < unboxingInfoMetadata.argsInfo.length; i++) {
+            if (unboxingInfoMetadata.argsInfo[i] == UnboxingType.kUnknown) {
+              unboxingInfoMetadata.argsInfo[i] = UnboxingType.kBoxed;
             }
           }
           if (!unboxingInfoMetadata.isFullyBoxed) {

pkg/vm/lib/transformations/type_flow/types.dart

@@ -24,16 +24,19 @@ import 'utils.dart';
 class TFClass {
   final int id;
   final Class classNode;
+  final RecordShape? recordShape;
 
   /// TFClass should not be instantiated directly.
   /// Instead, [TypeHierarchy.getTFClass] should be used to obtain [TFClass]
   /// instances specific to given [TypeHierarchy].
-  TFClass(this.id, this.classNode);
+  TFClass(this.id, this.classNode, this.recordShape);
 
   /// Returns ConcreteType corresponding to this class without
   /// any extra attributes.
   ConcreteType get concreteType => ConcreteType(this);
 
+  bool get isRecord => recordShape != null;
+
   @override
   int get hashCode => id;
 
@@ -54,7 +57,7 @@ class RecordShape {
       : numPositionalFields = type.positional.length,
         namedFields = type.named.isEmpty
             ? const <String>[]
-            : type.named.map((nt) => nt.name).toList();
+            : type.named.map((nt) => nt.name).toList(growable: false);
 
   int get numFields => numPositionalFields + namedFields.length;
 
@@ -66,6 +69,20 @@ class RecordShape {
     }
   }
 
+  RecordShape.readFromBinary(BinarySource source)
+      : numPositionalFields = source.readUInt30(),
+        namedFields = List<String>.generate(
+            source.readUInt30(), (_) => source.readStringReference(),
+            growable: false);
+
+  void writeToBinary(BinarySink sink) {
+    sink.writeUInt30(numPositionalFields);
+    sink.writeUInt30(namedFields.length);
+    for (int i = 0; i < namedFields.length; ++i) {
+      sink.writeStringReference(namedFields[i]);
+    }
+  }
+
   @override
   int get hashCode => (_hash == 0) ? _computeHashCode() : _hash;
 

pkg/vm/lib/transformations/type_flow/unboxing_info.dart

@@ -17,6 +17,10 @@ import '../../metadata/unboxing_info.dart';
 import 'utils.dart';
 
 class UnboxingInfoManager {
+  // Unbox records in return values if they have
+  // exactly this number of fields.
+  static const int numRecordFieldsForReturnValueUnboxing = 2;
+
   final Map<Member, UnboxingInfoMetadata> _memberInfo = {};
 
   final TypeHierarchy _typeHierarchy;
@@ -93,8 +97,8 @@ class UnboxingInfoManager {
           info = UnboxingInfoMetadata(paramCount);
           selectorIdToInfo[selectorId] = info;
         } else {
-          if (paramCount < info.unboxedArgsInfo.length) {
-            info.unboxedArgsInfo.length = paramCount;
+          if (paramCount < info.argsInfo.length) {
+            info.argsInfo.length = paramCount;
           }
         }
       } else {
@@ -121,8 +125,8 @@ class UnboxingInfoManager {
     if (typeFlowAnalysis.isMemberUsed(member)) {
       final UnboxingInfoMetadata unboxingInfo = _memberInfo[member]!;
       if (_cannotUnbox(member)) {
-        unboxingInfo.unboxedArgsInfo.length = 0;
-        unboxingInfo.returnInfo = UnboxingInfoMetadata.kBoxed;
+        unboxingInfo.argsInfo.length = 0;
+        unboxingInfo.returnInfo = UnboxingType.kBoxed;
         return;
       }
       if (member is Procedure || member is Constructor) {
@@ -150,52 +154,51 @@ class UnboxingInfoManager {
         final Type resultType = typeFlowAnalysis.getSummary(member).resultType;
         _applyToReturn(member, unboxingInfo, resultType);
       } else if (member is Field) {
-        final fieldValue = typeFlowAnalysis.getFieldValue(member).value;
+        final inferredType = typeFlowAnalysis.getFieldValue(member).value;
         if (member.hasSetter) {
-          _applyToArg(member, unboxingInfo, 0, fieldValue);
+          _applyToArg(member, unboxingInfo, 0, inferredType);
         }
-        _applyToReturn(member, unboxingInfo, fieldValue);
+        _applyToReturn(member, unboxingInfo, inferredType);
       } else {
         assert(false, "Unexpected member: $member");
       }
     }
   }
 
-  int _getUnboxingType(Member member, Type type, bool isReturn) {
+  UnboxingType _getUnboxingType(Member member, Type type, bool isReturn) {
     if (type is! NullableType) {
       if (type.isSubtypeOf(_typeHierarchy, _coreTypes.intClass)) {
-        return UnboxingInfoMetadata.kUnboxedIntCandidate;
+        return UnboxingType.kInt;
       }
       if (type.isSubtypeOf(_typeHierarchy, _coreTypes.doubleClass)) {
-        return UnboxingInfoMetadata.kUnboxedDoubleCandidate;
+        return UnboxingType.kDouble;
       }
       if (isReturn) {
-        final function = member.function;
-        if (function != null) {
-          final returnType = function.returnType;
-          if (returnType is RecordType &&
-              (returnType.positional.length + returnType.named.length) == 2) {
-            return UnboxingInfoMetadata.kUnboxedRecordCandidate;
-          }
+        if (type is ConcreteType &&
+            type.cls.isRecord &&
+            type.cls.recordShape!.numFields ==
+                numRecordFieldsForReturnValueUnboxing) {
+          return UnboxingType.record(type.cls.recordShape!);
         }
       }
     }
-    return UnboxingInfoMetadata.kBoxed;
+    return UnboxingType.kBoxed;
   }
 
   void _applyToArg(
       Member member, UnboxingInfoMetadata unboxingInfo, int argPos, Type type) {
-    if (argPos < 0 || unboxingInfo.unboxedArgsInfo.length <= argPos) {
+    if (argPos < 0 || unboxingInfo.argsInfo.length <= argPos) {
       return;
     }
     final unboxingType = _getUnboxingType(member, type, false);
-    unboxingInfo.unboxedArgsInfo[argPos] &= unboxingType;
+    unboxingInfo.argsInfo[argPos] =
+        unboxingInfo.argsInfo[argPos].intersect(unboxingType);
   }
 
   void _applyToReturn(
       Member member, UnboxingInfoMetadata unboxingInfo, Type type) {
     final unboxingType = _getUnboxingType(member, type, true);
-    unboxingInfo.returnInfo &= unboxingType;
+    unboxingInfo.returnInfo = unboxingInfo.returnInfo.intersect(unboxingType);
   }
 
   bool _cannotUnbox(Member member) {

pkg/vm/test/transformations/type_flow/summary_collector_test.dart

@@ -34,7 +34,7 @@ class FakeTypesBuilder extends TypesBuilder {
 
   @override
   TFClass getTFClass(Class c) =>
-      _classes[c] ??= new TFClass(++_classIdCounter, c);
+      _classes[c] ??= new TFClass(++_classIdCounter, c, null);
 }
 
 class FakeEntryPointsListener implements EntryPointsListener {